Skip to content

Commit

Permalink
feat(linter): do not report directive in commonjs files (#3732)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Aug 29, 2024
1 parent bf7fcb7 commit 33040d7
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,8 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

Contributed by @ematipico

- The rule `noRedundantUseStrict` no longer reports `use strict` when the `package.json` marks the file as a script using the field `"type": "commonjs"`. Contributed by @ematipico

#### Bug fixes

- Don't request alt text for elements hidden from assistive technologies ([#3316](https://github.com/biomejs/biome/issues/3316)). Contributed by @robintown
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use biome_analyze::{
};
use biome_console::markup;
use biome_js_syntax::{
AnyJsClass, JsDirective, JsDirectiveList, JsFunctionBody, JsModule, JsScript,
AnyJsClass, JsDirective, JsDirectiveList, JsFileSource, JsFunctionBody, JsModule, JsScript,
};

use biome_rowan::{declare_node_union, AstNode, BatchMutationExt};
use biome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutationExt};

declare_lint_rule! {
/// Prevents from having redundant `"use strict"`.
Expand Down Expand Up @@ -98,8 +98,12 @@ impl AnyNodeWithDirectives {
AnyNodeWithDirectives::JsScript(script) => script.directives(),
}
}

const fn is_script(&self) -> bool {
matches!(self, AnyNodeWithDirectives::JsScript(_))
}
}
declare_node_union! { pub AnyJsStrictModeNode = AnyJsClass| JsModule | JsDirective }
declare_node_union! { pub AnyJsStrictModeNode = AnyJsClass | JsModule | JsDirective }

impl Rule for NoRedundantUseStrict {
type Query = Ast<JsDirective>;
Expand All @@ -112,6 +116,7 @@ impl Rule for NoRedundantUseStrict {
if node.inner_string_text().ok()? != "use strict" {
return None;
}
let file_source = ctx.source_type::<JsFileSource>();
let mut outer_most: Option<AnyJsStrictModeNode> = None;
let root = ctx.root();
match root {
Expand All @@ -120,9 +125,19 @@ impl Rule for NoRedundantUseStrict {
for n in node.syntax().ancestors() {
match AnyNodeWithDirectives::try_cast(n) {
Ok(parent) => {
for directive in parent.directives() {
let directives_len = parent.directives().len();
for (index, directive) in parent.directives().into_iter().enumerate() {
let directive_text = directive.inner_string_text().ok()?;

if directive_text == "use strict" {
// if we are analysing a commonjs file, we ignore the first directive that we have at the top, because it's not redundant
if index + 1 == directives_len
&& parent.is_script()
&& file_source.is_script()
&& outer_most.is_none()
{
break;
}
outer_most = Some(directive.into());
break; // continue with next parent
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"use strict"

const a = require("a")
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: commonJsValid.js
---
# Input
```jsx
"use strict"

const a = require("a")

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "commonjs"
}
10 changes: 10 additions & 0 deletions crates/biome_project/src/node_js_project/package_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,13 @@ pub enum PackageType {
Module,
Commonjs,
}

impl PackageType {
pub const fn is_commonjs(&self) -> bool {
matches!(self, Self::Commonjs)
}

pub const fn is_module(&self) -> bool {
matches!(self, Self::Module)
}
}

0 comments on commit 33040d7

Please sign in to comment.