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): add module and classes support for noRedundant…
Browse files Browse the repository at this point in the history 
…UseStrict (#3955)

Co-authored-by: Micha Reiser <[email protected]>
  • Loading branch information
@mzbac @MichaReiser
mzbac and MichaReiser authored Dec 12, 2022
1 parent 287fcf9 commit 63409a8
Show file tree
Hide file tree
Showing 17 changed files with 680 additions and 125 deletions.
86 changes: 59 additions & 27 deletions crates/rome_js_analyze/src/analyzers/nursery/no_redundant_use_strict.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ 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_js_syntax::{
AnyJsClass, JsDirective, JsDirectiveList, JsFunctionBody, JsModule, JsScript,
};

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

Expand All @@ -12,33 +14,48 @@ declare_rule! {
/// ## Examples
///
/// ### Invalid
/// ```js,expect_diagnostic
/// ```cjs,expect_diagnostic
/// "use strict";
/// function foo() {
/// "use strict";
/// }
/// ```
/// ```js,expect_diagnostic
/// ```cjs,expect_diagnostic
/// "use strict";
/// "use strict";
///
/// function foo() {
///
/// }
/// ```
/// ```js,expect_diagnostic
/// ```cjs,expect_diagnostic
/// function foo() {
/// "use strict";
/// "use strict";
/// }
/// ```
/// ```cjs,expect_diagnostic
/// class C1 {
/// test() {
/// "use strict";
/// }
/// }
/// ```
/// ```cjs,expect_diagnostic
/// const C2 = class {
/// test() {
/// "use strict";
/// }
/// };
///
/// ```
/// ### Valid
/// ```js
/// ```cjs
/// function foo() {
///
/// }
///```
/// ```js
/// ```cjs
/// function foo() {
/// "use strict";
/// }
Expand All @@ -55,44 +72,49 @@ declare_rule! {
}
}

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

impl Rule for NoRedundantUseStrict {
type Query = Ast<JsDirective>;
type State = JsDirective;
type State = AnyJsStrictModeNode;
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
let mut outer_most: Option<AnyJsStrictModeNode> = None;
let root = ctx.root();
match root {
rome_js_syntax::AnyJsRoot::JsModule(js_module) => outer_most = Some(js_module.into()),
_ => {
for n in node.syntax().ancestors() {
if let Some(parent) = AnyNodeWithDirectives::cast_ref(&n) {
for directive in parent.directives() {
let directive_text = directive.inner_string_text().ok()?;
if directive_text == "use strict" {
outer_most = Some(directive.into());
break; // continue with next parent
}
}
} else if let Some(module_or_class) = AnyJsClass::cast_ref(&n) {
outer_most = Some(module_or_class.into());
}
}
}
}

if let Some(outer_most) = outer_most {
// skip itself
if &outer_most == node {
if outer_most.syntax() == node.syntax() {
return None;
}
return Some(outer_most);
Expand All @@ -101,18 +123,28 @@ impl Rule for NoRedundantUseStrict {
None
}
fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let diag = RuleDiagnostic::new(
let mut 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."},
);

match state {
AnyJsStrictModeNode::AnyJsClass(js_class) => diag = diag.detail(
js_class.range(),
markup! {"All parts of a class's body are already in strict mode."},
) ,
AnyJsStrictModeNode::JsModule(_js_module) => diag= diag.note(
markup! {"The entire contents of "<Emphasis>{"JavaScript modules"}</Emphasis>" are automatically in strict mode, with no statement needed to initiate it."},
),
AnyJsStrictModeNode::JsDirective(js_directive) => diag= diag.detail(
js_directive.range(),
markup! {"This outer "<Emphasis>{"use strict"}</Emphasis>" directive already enables strict mode."},
),
}

Some(diag)
}

Expand Down
14 changes: 14 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/noRedundantUseStrict/invalid.cjs
View file
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
}
}
}
149 changes: 149 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/noRedundantUseStrict/invalid.cjs.snap
View file
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: 86
expression: invalid.cjs
---
# 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.cjs: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.cjs: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.cjs: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.cjs: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 │ }
```


24 changes: 15 additions & 9 deletions crates/rome_js_analyze/tests/specs/nursery/noRedundantUseStrict/invalid.js
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
"use strict";
// js module
"use strict";

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

class C1 {
// All code here is evaluated in strict mode
test() {
"use strict";
}
}

const C2 = class {
// All code here is evaluated in strict mode
test() {
"use strict";
}
};
Loading

0 comments on commit 63409a8

Please sign in to comment.