Skip to content

Commit

Permalink
feat(lint): add variable assignment tracking to noDocumentCookie ru…
Browse files Browse the repository at this point in the history
…le (#4255)
  • Loading branch information
tunamaguro authored Oct 11, 2024
1 parent 8b3c102 commit 773f5b0
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 18 deletions.
38 changes: 30 additions & 8 deletions crates/biome_js_analyze/src/lint/nursery/no_document_cookie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use biome_analyze::{context::RuleContext, declare_lint_rule, Rule, RuleDiagnosti
use biome_console::markup;
use biome_js_semantic::SemanticModel;
use biome_js_syntax::{
global_identifier, AnyJsAssignment, AnyJsExpression, JsAssignmentExpression,
binding_ext::AnyJsBindingDeclaration, global_identifier, static_value::StaticValue,
AnyJsAssignment, AnyJsExpression, JsAssignmentExpression, JsReferenceIdentifier,
};
use biome_rowan::AstNode;

Expand Down Expand Up @@ -57,17 +58,37 @@ declare_lint_rule! {
}
}

fn identifier_is_global_document(
reference: &JsReferenceIdentifier,
name: &StaticValue,
model: &SemanticModel,
) -> bool {
// Check identifier is `document` and global
name.text() == "document" && model.binding(reference).is_none()
}

/// Check `expr` is `document`
fn is_global_document(expr: &AnyJsExpression, model: &SemanticModel) -> Option<()> {
let (reference, name) = global_identifier(expr)?;

// Check identifier is `document`
if name.text() != "document" {
return None;
};

// TODO: Verify that the variable is assigned the global `document` to be closer to the original rule.
model.binding(&reference).is_none().then_some(())
// `expr` is global document
if identifier_is_global_document(&reference, &name, model) {
Some(())
} else {
// Check binding declaration recursively
let bind = model.binding(&reference)?;
let decl = bind.tree().declaration()?;
let decl = decl.parent_binding_pattern_declaration().unwrap_or(decl);
match decl {
// const foo = documnet;
AnyJsBindingDeclaration::JsVariableDeclarator(declarator) => {
let initializer = declarator.initializer()?;
let right_expr = initializer.expression().ok()?;
is_global_document(&right_expr, model)
}
_ => None,
}
}
}

/// Check member is `cookie`
Expand Down Expand Up @@ -110,6 +131,7 @@ impl Rule for NoDocumentCookie {

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();

let left = node.left().ok()?;

let any_assignment = left.as_any_js_assignment()?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,20 @@ document.cookie += ";foo=bar"
window.document.cookie = "foo=bar";
globalThis.window.document.cookie = "foo=bar";

document["cookie"] = "foo=bar"
document["cookie"] = "foo=bar"

function document_is_global1(){
const doc = document;
doc.cookie = "bar=foo"
}

function document_is_global2(){
const foo = window.document;
const bar = foo;
bar.cookie = "foo=bar";
}

const global_doc = globalThis.document;
function document_is_global3(){
global_doc.cookie = "foo=bar";
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@ window.document.cookie = "foo=bar";
globalThis.window.document.cookie = "foo=bar";

document["cookie"] = "foo=bar"

function document_is_global1(){
const doc = document;
doc.cookie = "bar=foo"
}

function document_is_global2(){
const foo = window.document;
const bar = foo;
bar.cookie = "foo=bar";
}

const global_doc = globalThis.document;
function document_is_global3(){
global_doc.cookie = "foo=bar";
}
```

# Diagnostics
Expand Down Expand Up @@ -87,6 +103,58 @@ invalid.js:7:1 lint/nursery/noDocumentCookie ━━━━━━━━━━━
6 │
> 7 │ document["cookie"] = "foo=bar"
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8 │
9 │ function document_is_global1(){
i Consider using the Cookie Store API.
```

```
invalid.js:11:5 lint/nursery/noDocumentCookie ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Direct assigning to document.cookie is not recommended.
9 │ function document_is_global1(){
10 │ const doc = document;
> 11 │ doc.cookie = "bar=foo"
│ ^^^^^^^^^^^^^^^^^^^^^^
12 │ }
13 │
i Consider using the Cookie Store API.
```

```
invalid.js:17:5 lint/nursery/noDocumentCookie ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Direct assigning to document.cookie is not recommended.
15 │ const foo = window.document;
16 │ const bar = foo;
> 17 │ bar.cookie = "foo=bar";
│ ^^^^^^^^^^^^^^^^^^^^^^
18 │ }
19 │
i Consider using the Cookie Store API.
```

```
invalid.js:22:5 lint/nursery/noDocumentCookie ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Direct assigning to document.cookie is not recommended.
20 │ const global_doc = globalThis.document;
21 │ function document_is_global3(){
> 22 │ global_doc.cookie = "foo=bar";
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23 │ }
i Consider using the Cookie Store API.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,3 @@ cookieStore
function document_is_not_global1(document){
document.cookie = "bar=foo"
}

function document_is_not_global2(){
const document = "foo";
document.cookie = "bar=foo"
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,4 @@ function document_is_not_global1(document){
document.cookie = "bar=foo"
}

function document_is_not_global2(){
const document = "foo";
document.cookie = "bar=foo"
}
```

0 comments on commit 773f5b0

Please sign in to comment.