Skip to content

Commit

Permalink
fix(linter): no-global-assign look for globals entry too
Browse files Browse the repository at this point in the history
  • Loading branch information
Sysix committed Feb 8, 2025
1 parent 41ad42a commit 7b10cad
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 21 deletions.
30 changes: 30 additions & 0 deletions crates/oxc_linter/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,19 @@ impl<'a> LintContext<'a> {
&& !self.globals().get(name).is_some_and(|value| *value == GlobalValue::Off)
}

/// Checks if the provided identifier is a reference to a global variable.
pub fn get_global_variable_value(&self, name: &str) -> Option<GlobalValue> {
if !self.scopes().root_unresolved_references().contains_key(name) {
return None;
}

if let Some(value) = self.globals().get(name) {
return Some(*value);
}

self.get_env_global_entry(name)
}

/// Runtime environments turned on/off by the user.
///
/// Examples of environments are `builtin`, `browser`, `node`, etc.
Expand All @@ -165,6 +178,23 @@ impl<'a> LintContext<'a> {
&self.parent.config.env
}

fn get_env_global_entry(&self, var: &str) -> Option<GlobalValue> {
// builtin is always readonly
if GLOBALS["builtin"].contains_key(var) {
return Some(GlobalValue::Readonly);
}

for env in self.env().iter() {
if let Some(env) = GLOBALS.get(env) {
if let Some(value) = env.get(var) {
return Some(GlobalValue::from(*value));
};
}
}

None
}

/// Checks if a given variable named is defined as a global variable in the current environment.
///
/// Example:
Expand Down
17 changes: 13 additions & 4 deletions crates/oxc_linter/src/javascript_globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"Int32Array" => false,
"Int8Array" => false,
"Intl" => false,
"Iterator" => false,
"JSON" => false,
"Map" => false,
"Math" => false,
Expand Down Expand Up @@ -432,6 +433,7 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"CSSKeywordValue" => false,
"CSSLayerBlockRule" => false,
"CSSLayerStatementRule" => false,
"CSSMarginRule" => false,
"CSSMathClamp" => false,
"CSSMathInvert" => false,
"CSSMathMax" => false,
Expand All @@ -443,6 +445,7 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"CSSMatrixComponent" => false,
"CSSMediaRule" => false,
"CSSNamespaceRule" => false,
"CSSNestedDeclarations" => false,
"CSSNumericArray" => false,
"CSSNumericValue" => false,
"CSSPageDescriptors" => false,
Expand Down Expand Up @@ -754,7 +757,6 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"InputEvent" => false,
"IntersectionObserver" => false,
"IntersectionObserverEntry" => false,
"Iterator" => false,
"Keyboard" => false,
"KeyboardEvent" => false,
"KeyboardLayoutMap" => false,
Expand Down Expand Up @@ -1216,12 +1218,15 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"XRDOMOverlayState" => false,
"XRDepthInformation" => false,
"XRFrame" => false,
"XRHand" => false,
"XRHitTestResult" => false,
"XRHitTestSource" => false,
"XRInputSource" => false,
"XRInputSourceArray" => false,
"XRInputSourceEvent" => false,
"XRInputSourcesChangeEvent" => false,
"XRJointPose" => false,
"XRJointSpace" => false,
"XRLayer" => false,
"XRLightEstimate" => false,
"XRLightProbe" => false,
Expand Down Expand Up @@ -1489,6 +1494,7 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"BroadcastChannel" => false,
"Buffer" => false,
"ByteLengthQueuingStrategy" => false,
"CloseEvent" => false,
"CompressionStream" => false,
"CountQueuingStrategy" => false,
"Crypto" => false,
Expand All @@ -1501,7 +1507,6 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"File" => false,
"FormData" => false,
"Headers" => false,
"Iterator" => false,
"MessageChannel" => false,
"MessageEvent" => false,
"MessagePort" => false,
Expand Down Expand Up @@ -1564,6 +1569,7 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"Blob" => false,
"BroadcastChannel" => false,
"ByteLengthQueuingStrategy" => false,
"CloseEvent" => false,
"CompressionStream" => false,
"CountQueuingStrategy" => false,
"Crypto" => false,
Expand All @@ -1576,7 +1582,6 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"File" => false,
"FormData" => false,
"Headers" => false,
"Iterator" => false,
"MessageChannel" => false,
"MessageEvent" => false,
"MessagePort" => false,
Expand Down Expand Up @@ -1718,6 +1723,10 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"GPUTextureView" => false,
"GPUUncapturedErrorEvent" => false,
"GPUValidationError" => false,
"HID" => false,
"HIDConnectionEvent" => false,
"HIDDevice" => false,
"HIDInputReportEvent" => false,
"Headers" => false,
"IDBCursor" => false,
"IDBCursorWithValue" => false,
Expand All @@ -1737,7 +1746,6 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"ImageDecoder" => false,
"ImageTrack" => false,
"ImageTrackList" => false,
"Iterator" => false,
"Lock" => false,
"LockManager" => false,
"MediaCapabilities" => false,
Expand Down Expand Up @@ -1772,6 +1780,7 @@ pub static GLOBALS: Map<&'static str, Map<&'static str, bool>> = phf_map! {
"PushManager" => false,
"PushSubscription" => false,
"PushSubscriptionOptions" => false,
"RTCDataChannel" => false,
"RTCEncodedAudioFrame" => false,
"RTCEncodedVideoFrame" => false,
"ReadableByteStreamController" => false,
Expand Down
68 changes: 51 additions & 17 deletions crates/oxc_linter/src/rules/eslint/no_global_assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::{CompactStr, Span};

use crate::{context::LintContext, rule::Rule};
use crate::{config::GlobalValue, context::LintContext, rule::Rule};

fn no_global_assign_diagnostic(global_name: &str, span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("Read-only global '{global_name}' should not be modified."))
Expand Down Expand Up @@ -65,7 +65,9 @@ impl Rule for NoGlobalAssign {
let reference = symbol_table.get_reference(reference_id);
if reference.is_write()
&& !self.excludes.iter().any(|n| n == name)
&& ctx.env_contains_var(name)
&& ctx
.get_global_variable_value(name)
.is_some_and(|global| global == GlobalValue::Readonly)
{
ctx.diagnostic(no_global_assign_diagnostic(
name,
Expand All @@ -82,28 +84,60 @@ fn test() {
use crate::tester::Tester;

let pass = vec![
("string='1';", None),
("var string;", None),
("Object = 0;", Some(serde_json::json!([{ "exceptions": ["Object"] }]))),
("top = 0;", None),
// ("onload = 0;", None), // env: { browser: true }
("require = 0;", None),
("window[parseInt('42', 10)] = 99;", None),
// ("a = 1", None), // globals: { a: true } },
("string='1';", None, None),
("var string;", None, None),
("Object = 0;", Some(serde_json::json!([{ "exceptions": ["Object"] }])), None),
("top = 0;", None, None),
(
"onload = 0;",
None,
Some(serde_json::json!({
"env": {
"browser": true
}
})),
),
("require = 0;", None, None),
("window[parseInt('42', 10)] = 99;", None, None),
(
"a = 1",
None,
Some(serde_json::json!({
"globals": {
"a": true
}
})),
),
// ("/*global a:true*/ a = 1", None),
];

let fail = vec![
("String = 'hello world';", None),
("String++;", None),
("({Object = 0, String = 0} = {});", None),
// ("top = 0;", None), // env: { browser: true },
// ("require = 0;", None), // env: { node: true },
("function f() { Object = 1; }", None),
("String = 'hello world';", None, None),
("String++;", None, None),
("({Object = 0, String = 0} = {});", None, None),
(
"top = 0;",
None,
Some(serde_json::json!({
"env": {
"browser": true
}
})),
),
(
"require = 0;",
None,
Some(serde_json::json!({
"env": {
"node": true
}
})),
),
("function f() { Object = 1; }", None, None),
// ("/*global b:false*/ function f() { b = 1; }", None),
// ("/*global b:false*/ function f() { b++; }", None),
// ("/*global b*/ b = 1;", None),
("Array = 1;", None),
("Array = 1;", None, None),
];

Tester::new(NoGlobalAssign::NAME, NoGlobalAssign::PLUGIN, pass, fail).test_and_snapshot();
Expand Down
14 changes: 14 additions & 0 deletions crates/oxc_linter/src/snapshots/eslint_no_global_assign.snap
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,20 @@ source: crates/oxc_linter/src/tester.rs
· ╰── Read-only global 'Object' should not be modified.
╰────

eslint(no-global-assign): Read-only global 'top' should not be modified.
╭─[no_global_assign.tsx:1:1]
1top = 0;
· ─┬─
· ╰── Read-only global 'top' should not be modified.
╰────

eslint(no-global-assign): Read-only global 'require' should not be modified.
╭─[no_global_assign.tsx:1:1]
1require = 0;
· ───┬───
· ╰── Read-only global 'require' should not be modified.
╰────

eslint(no-global-assign): Read-only global 'Object' should not be modified.
╭─[no_global_assign.tsx:1:16]
1function f() { Object = 1; }
Expand Down

0 comments on commit 7b10cad

Please sign in to comment.