Skip to content

Commit

Permalink
chore(linter): mark many rules as fix-pending (#4723)
Browse files Browse the repository at this point in the history
Many rules still need `pending` flags, particularly the unicorn rules.

I also enhanced documentation for some rules while I was in the area.
  • Loading branch information
DonIsaac committed Aug 7, 2024
1 parent 3289477 commit c296373
Show file tree
Hide file tree
Showing 23 changed files with 301 additions and 31 deletions.
1 change: 1 addition & 0 deletions crates/oxc_linter/src/rules/eslint/no_empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ declare_oxc_lint!(
/// ```
NoEmpty,
restriction,
pending // a suggestion could be added to remove the empty statement
);

impl Rule for NoEmpty {
Expand Down
5 changes: 4 additions & 1 deletion crates/oxc_linter/src/rules/eslint/no_extra_boolean_cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ declare_oxc_lint!(
/// if (!!foo || bar) {}
/// ```
NoExtraBooleanCast,
correctness
correctness,
// a suggestion could be added. Note that lacking !! can mess up TS type
// narrowing sometimes, so it should not be an autofix
pending
);

impl Rule for NoExtraBooleanCast {
Expand Down
171 changes: 170 additions & 1 deletion crates/oxc_linter/src/rules/eslint/no_fallthrough.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,177 @@ declare_oxc_lint!(
///
/// Disallow fallthrough of `case` statements
///
/// This rule is aimed at eliminating unintentional fallthrough of one case
/// to the other. As such, it flags any fallthrough scenarios that are not
/// marked by a comment.
///
/// ### Why is this bad?
///
/// The switch statement in JavaScript is one of the more error-prone
/// constructs of the language thanks in part to the ability to “fall
/// through” from one case to the next. For example:
///
/// ```js
/// switch(foo) {
/// case 1:
/// doSomething();
///
/// case 2:
/// doSomethingElse();
/// }
/// ```
///
/// In this example, if `foo` is `1`, then execution will flow through both
/// cases, as the first falls through to the second. You can prevent this by
/// using `break`, as in this example:
///
/// ```js
/// switch(foo) {
/// case 1:
/// doSomething();
/// break;
///
/// case 2:
/// doSomethingElse();
/// }
/// ```
///
/// That works fine when you don’t want a fallthrough, but what if the
/// fallthrough is intentional, there is no way to indicate that in the
/// language. It’s considered a best practice to always indicate when a
/// fallthrough is intentional using a comment which matches the
/// `/falls?\s?through/i`` regular expression but isn’t a directive:
///
/// ```js
/// switch(foo) {
/// case 1:
/// doSomething();
/// // falls through
///
/// case 2:
/// doSomethingElse();
/// }
///
/// switch(foo) {
/// case 1:
/// doSomething();
/// // fall through
///
/// case 2:
/// doSomethingElse();
/// }
///
/// switch(foo) {
/// case 1:
/// doSomething();
/// // fallsthrough
///
/// case 2:
/// doSomethingElse();
/// }
///
/// switch(foo) {
/// case 1: {
/// doSomething();
/// // falls through
/// }
///
/// case 2: {
/// doSomethingElse();
/// }
/// }
/// ```
///
/// In this example, there is no confusion as to the expected behavior. It
/// is clear that the first case is meant to fall through to the second
/// case.
///
/// ### Example
///
/// Examples of **incorrect** code for this rule:
/// ```js
/// /*oxlint no-fallthrough: "error"*/
///
/// switch(foo) {
/// case 1:
/// doSomething();
///
/// case 2:
/// doSomething();
/// }
/// ```
///
/// Examples of **correct** code for this rule:
/// ```js
/// /*oxlint no-fallthrough: "error"*/
///
/// switch(foo) {
/// case 1:
/// doSomething();
/// break;
///
/// case 2:
/// doSomething();
/// }
///
/// function bar(foo) {
/// switch(foo) {
/// case 1:
/// doSomething();
/// return;
///
/// case 2:
/// doSomething();
/// }
/// }
///
/// switch(foo) {
/// case 1:
/// doSomething();
/// throw new Error("Boo!");
///
/// case 2:
/// doSomething();
/// }
///
/// switch(foo) {
/// case 1:
/// case 2:
/// doSomething();
/// }
///
/// switch(foo) {
/// case 1: case 2:
/// doSomething();
/// }
///
/// switch(foo) {
/// case 1:
/// doSomething();
/// // falls through
///
/// case 2:
/// doSomething();
/// }
///
/// switch(foo) {
/// case 1: {
/// doSomething();
/// // falls through
/// }
///
/// case 2: {
/// doSomethingElse();
/// }
/// }
/// ```
///
/// Note that the last case statement in these examples does not cause a
/// warning because there is nothing to fall through into.
NoFallthrough,
pedantic // Fall through code are still incorrect.
// TODO: add options section to docs
pedantic, // Fall through code are still incorrect.
pending // TODO: add a dangerous suggestion for this rule.
);

impl Rule for NoFallthrough {
Expand Down
6 changes: 5 additions & 1 deletion crates/oxc_linter/src/rules/eslint/no_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ declare_oxc_lint!(
/// Disallow the use of the __iterator__ property
///
/// ### Why is this bad?
/// The __iterator__ property was a SpiderMonkey extension to JavaScript that could be used to create custom iterators that are compatible with JavaScript’s for in and for each constructs. However, this property is now obsolete, so it should not be used. Here’s an example of how this used to work:
/// The __iterator__ property was a SpiderMonkey extension to JavaScript
/// that could be used to create custom iterators that are compatible with
/// JavaScript’s for in and for each constructs. However, this property is
/// now obsolete, so it should not be used. Here’s an example of how this
/// used to work:
///
/// ### Example
/// ```javascript
Expand Down
21 changes: 20 additions & 1 deletion crates/oxc_linter/src/rules/eslint/no_label_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ declare_oxc_lint!(
/// that shares a name with a variable that is in scope.
///
/// ### Example
/// ```javascript
///
/// Examples of **incorrect** code for this rule:
///
/// ```js
/// var x = foo;
/// function bar() {
/// x:
Expand All @@ -36,6 +39,22 @@ declare_oxc_lint!(
/// }
/// }
/// ```
/// Examples of **correct** code for this rule:
///
/// ```js
/// // The variable that has the same name as the label is not in scope.
///
/// function foo() {
/// var q = t;
/// }
///
/// function bar() {
/// q:
/// for(;;) {
/// break q;
/// }
/// }
/// ```
NoLabelVar,
style,
);
Expand Down
10 changes: 7 additions & 3 deletions crates/oxc_linter/src/rules/eslint/no_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ pub struct NoVar;

declare_oxc_lint!(
/// ### What it does
/// ECMAScript 6 allows programmers to create variables with block scope instead of function scope using the `let` and `const` keywords.
/// Block scope is common in many other programming languages and helps programmers avoid mistakes
///
/// ECMAScript 6 allows programmers to create variables with block scope
/// instead of function scope using the `let` and `const` keywords. Block
/// scope is common in many other programming languages and helps
/// programmers avoid mistakes.
///
/// ### Why is this bad?
/// Using `var` in an es6 environment triggers this error
Expand All @@ -37,7 +40,8 @@ declare_oxc_lint!(
/// const CONFIG = {};
/// ```
NoVar,
restriction
restriction,
pending // TODO: add suggestion that replaces `var` with `let` or `const` depending on if its written to
);

impl Rule for NoVar {
Expand Down
43 changes: 40 additions & 3 deletions crates/oxc_linter/src/rules/eslint/require_await.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,52 @@ fn require_await_diagnostic(span0: Span) -> OxcDiagnostic {

declare_oxc_lint!(
/// ### What it does
/// Disallow async functions which have no await expression.
///
/// Disallow async functions which have no `await` expression.
///
/// ### Why is this bad?
///
/// Asynchronous functions in JavaScript behave differently than other
/// functions in two important ways:
///
/// 1. The return value is always a `Promise`.
/// 2. You can use the `await` operator inside of them.
///
/// The primary reason to use asynchronous functions is typically to use the
/// await operator, such as this:
///
/// ```js
/// async function fetchData(processDataItem) {
/// const response = await fetch(DATA_URL);
/// const data = await response.json();
///
/// return data.map(processDataItem);
/// }
/// ```
/// Asynchronous functions that don’t use await might not need to be
/// asynchronous functions and could be the unintentional result of
/// refactoring.
///
/// Note: this rule ignores async generator functions. This is because
/// generators yield rather than return a value and async generators might
/// yield all the values of another async generator without ever actually
/// needing to use await.
///
/// ### Example
/// ```javascript
///
/// Examples of **incorrect** code for this rule:
///
/// ```js
/// async function foo() {
/// doSomething();
/// }
/// ```
///
/// Examples of **correct** code for this rule:
///
/// ```js
/// async function foo() {
/// doSomething();
/// await doSomething();
/// }
/// ```
RequireAwait,
Expand Down
17 changes: 17 additions & 0 deletions crates/oxc_linter/src/rules/eslint/symbol_description.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,27 @@ declare_oxc_lint!(
///
/// The Symbol function may have an optional description.
///
/// ```js
/// var foo = Symbol("some description");
///
/// var someString = "some description";
/// var bar = Symbol(someString);
/// ```
///
/// Using `description` promotes easier debugging: when a symbol is logged the description is used:
/// ```js
/// var foo = Symbol("some description");
///
/// console.log(foo);
/// // prints - Symbol(some description)
/// ```
///
/// ### Example
/// ```javascript
/// var foo = Symbol();
/// ```
///
///
SymbolDescription,
pedantic,
);
Expand Down
16 changes: 9 additions & 7 deletions crates/oxc_linter/src/rules/eslint/valid_typeof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,20 @@ declare_oxc_lint!(
/// Enforce comparing `typeof` expressions against valid strings
///
/// ### Why is this bad?
/// It is usually a typing mistake to compare the result of a typeof operator to other string literals.
/// It is usually a typing mistake to compare the result of a `typeof`
/// operator to other string literals.
///
/// ### Example
/// ```javascript
/// requireStringLiterals: false
/// incorrect:
/// ```js
/// // requireStringLiterals: false
/// // incorrect:
/// typeof foo === "strnig"
/// correct:
/// // correct:
/// typeof foo === "string"
/// typeof foo === baz
///
/// requireStringLiterals: true
/// incorrect:
/// // requireStringLiterals: true
/// // incorrect:
/// typeof foo === baz
/// ```
ValidTypeof,
Expand Down
3 changes: 2 additions & 1 deletion crates/oxc_linter/src/rules/oxc/missing_throw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ declare_oxc_lint!(
/// const foo = () => { new Error() }
/// ```
MissingThrow,
correctness
correctness,
pending // TODO: add a suggestion that adds `throw`
);

impl Rule for MissingThrow {
Expand Down
Loading

0 comments on commit c296373

Please sign in to comment.