Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
feat(rome_js_analyze): noRedundantUseStrict rule (#3895)
Browse files Browse the repository at this point in the history
* wip

* add docs

* update config and docs

* update tests

* fix lint

* refactor: change the impl based on feedback

* remove dbg

* refactor: address comments

* refactor: change from remove token to remove node

* address comments

* push case change

* Delete NoRedundantUseStrict.md

* rename to temp folder name

* rename back

* add lint doc back

* address comments and and test cases

* update lint doc

* Update crates/rome_js_analyze/src/analyzers/nursery/no_redundant_use_strict.rs

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

* Update crates/rome_js_analyze/src/analyzers/nursery/no_redundant_use_strict.rs

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

* update doc

* fix conflicts

* fix code gen

Co-authored-by: Anchen Li <anchen.li@pslm-qxy9gygyvj.lan>
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Co-authored-by: Anchen Li <anchen.li@PSLM-QXY9GYGYVJ.local>
  • Loading branch information
4 people committed Nov 30, 2022
1 parent e7f6be8 commit c595ab0
Show file tree
Hide file tree
Showing 15 changed files with 548 additions and 4 deletions.
6 changes: 4 additions & 2 deletions crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ define_dategories! {
"lint/nursery/useValidForDirection": "https://docs.rome.tools/lint/rules/useValidForDirection",
"lint/nursery/useAriaPropsForRole": "https://docs.rome.tools/lint/rules/useAriaPropsForRole",
"lint/nursery/useAriaPropTypes": "https://docs.rome.tools/lint/rules/useAriaPropTypes",
"lint/nursery/noRedundantUseStrict": "https://docs.rome.tools/lint/rules/noRedundantUseStrict",

;

// General categories
Expand All @@ -115,7 +117,7 @@ define_dategories! {
// parse categories
"parse",
"parse/noSuperWithoutExtends",

// Lint groups
"lint",
"lint/correctness",
Expand All @@ -125,7 +127,7 @@ define_dategories! {
"lint/security",
"lint/nursery",
"lint/configuration",

// Suppression comments
"suppressions/parse",
"suppressions/unknownGroup",
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
use crate::JsRuleAction;
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_syntax::{JsDirective, JsDirectiveList, JsFunctionBody, JsModule, JsScript};

use rome_rowan::{declare_node_union, AstNode, BatchMutationExt};

declare_rule! {
/// Prevents from having redundant `"use strict"`.
///
/// ## Examples
///
/// ### Invalid
/// ```js,expect_diagnostic
/// "use strict";
/// function foo() {
/// "use strict";
/// }
/// ```
/// ```js,expect_diagnostic
/// "use strict";
/// "use strict";
///
/// function foo() {
///
/// }
/// ```
/// ```js,expect_diagnostic
/// function foo() {
/// "use strict";
/// "use strict";
/// }
/// ```
/// ### Valid
/// ```js
/// function foo() {
///
/// }
///```
/// ```js
/// function foo() {
/// "use strict";
/// }
/// function bar() {
/// "use strict";
/// }
///```
///

pub(crate) NoRedundantUseStrict {
version: "11.0.0",
name: "noRedundantUseStrict",
recommended: false,
}
}

declare_node_union! { AnyNodeWithDirectives = JsFunctionBody | JsModule | JsScript }
impl AnyNodeWithDirectives {
fn directives(&self) -> JsDirectiveList {
match self {
AnyNodeWithDirectives::JsFunctionBody(node) => node.directives(),
AnyNodeWithDirectives::JsScript(script) => script.directives(),
AnyNodeWithDirectives::JsModule(module) => module.directives(),
}
}
}

impl Rule for NoRedundantUseStrict {
type Query = Ast<JsDirective>;
type State = JsDirective;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
let mut outer_most: Option<JsDirective> = None;
for parent in node
.syntax()
.ancestors()
.filter_map(AnyNodeWithDirectives::cast)
{
for directive in parent.directives() {
if directive.value_token().map_or(false, |t| {
matches!(t.text_trimmed(), "'use strict'" | "\"use strict\"")
}) {
outer_most = Some(directive);
break; // continue with next parent
}
}
}

if let Some(outer_most) = outer_most {
// skip itself
if &outer_most == node {
return None;
}
return Some(outer_most);
}

None
}
fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let diag = RuleDiagnostic::new(
rule_category!(),
ctx.query().range(),
markup! {
"Redundant "<Emphasis>{"use strict"}</Emphasis>" directive."
},
)
.detail(
state.range(),
markup! {"This outer "<Emphasis>{"use strict"}</Emphasis>" directive already enables strict mode."},
);

Some(diag)
}

fn action(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<JsRuleAction> {
let root = ctx.root();
let mut batch = root.begin();

batch.remove_node(ctx.query().clone());

Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::Always,
message: markup! { "Remove the redundant \"use strict\" directive" }.to_owned(),
mutation: batch,
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
"use strict";
"use strict";

function test() {
"use strict";
function inner_a() {
"use strict"; // redundant directive
}
function inner_b() {
function inner_inner() {
"use strict"; // additional redundant directive
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
assertion_line: 83
expression: invalid.js
---
# Input
```js
"use strict";
"use strict";

function test() {
"use strict";
function inner_a() {
"use strict"; // redundant directive
}
function inner_b() {
function inner_inner() {
"use strict"; // additional redundant directive
}
}
}

```

# Diagnostics
```
invalid.js:2:1 lint/nursery/noRedundantUseStrict FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Redundant use strict directive.
1 │ "use strict";
> 2 │ "use strict";
│ ^^^^^^^^^^^^^
3 │
4 │ function test() {
i This outer use strict directive already enables strict mode.
> 1 │ "use strict";
│ ^^^^^^^^^^^^^
2 │ "use strict";
3 │
i Safe fix: Remove the redundant "use strict" directive
1 1 │ "use strict";
2 │ - "use·strict";
3 2 │
4 3 │ function test() {
```

```
invalid.js:5:2 lint/nursery/noRedundantUseStrict FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Redundant use strict directive.
4 │ function test() {
> 5 │ "use strict";
│ ^^^^^^^^^^^^^
6 │ function inner_a() {
7 │ "use strict"; // redundant directive
i This outer use strict directive already enables strict mode.
> 1 │ "use strict";
│ ^^^^^^^^^^^^^
2 │ "use strict";
3 │
i Safe fix: Remove the redundant "use strict" directive
3 3 │
4 4 │ function test() {
5 │ - → "use·strict";
6 │ - → function·inner_a()·{
5 │ + → function·inner_a()·{
7 6 │ "use strict"; // redundant directive
8 7 │ }
```

```
invalid.js:7:3 lint/nursery/noRedundantUseStrict FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Redundant use strict directive.
5 │ "use strict";
6 │ function inner_a() {
> 7 │ "use strict"; // redundant directive
│ ^^^^^^^^^^^^^
8 │ }
9 │ function inner_b() {
i This outer use strict directive already enables strict mode.
> 1 │ "use strict";
│ ^^^^^^^^^^^^^
2 │ "use strict";
3 │
i Safe fix: Remove the redundant "use strict" directive
5 5 │ "use strict";
6 6 │ function inner_a() {
7 │ - → → "use·strict";·//·redundant·directive
8 │ - → }
7 │ + → }
9 8 │ function inner_b() {
10 9 │ function inner_inner() {
```

```
invalid.js:11:4 lint/nursery/noRedundantUseStrict FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Redundant use strict directive.
9 │ function inner_b() {
10 │ function inner_inner() {
> 11 │ "use strict"; // additional redundant directive
│ ^^^^^^^^^^^^^
12 │ }
13 │ }
i This outer use strict directive already enables strict mode.
> 1 │ "use strict";
│ ^^^^^^^^^^^^^
2 │ "use strict";
3 │
i Safe fix: Remove the redundant "use strict" directive
9 9 │ function inner_b() {
10 10 │ function inner_inner() {
11 │ - → → → "use·strict";·//·additional·redundant·directive
12 │ - → → }
11 │ + → → }
13 12 │ }
14 13 │ }
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
function test() {
"use strict";
"use strict";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
assertion_line: 83
expression: invalidFunction.js
---
# Input
```js
function test() {
"use strict";
"use strict";
}

```

# Diagnostics
```
invalidFunction.js:3:2 lint/nursery/noRedundantUseStrict FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Redundant use strict directive.
1 │ function test() {
2 │ "use strict";
> 3 │ "use strict";
│ ^^^^^^^^^^^^^
4 │ }
5 │
i This outer use strict directive already enables strict mode.
1 │ function test() {
> 2 │ "use strict";
│ ^^^^^^^^^^^^^
3 │ "use strict";
4 │ }
i Safe fix: Remove the redundant "use strict" directive
1 1 │ function test() {
2 2 │ "use strict";
3 │ - → "use·strict";
4 3 │ }
5 4 │
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
function foo() {
"use strict";
}
function bar() {
"use strict";
}
Loading

0 comments on commit c595ab0

Please sign in to comment.