Skip to content

Commit

Permalink
fix(linter): correct the is_reference_to_global_variable (#8920)
Browse files Browse the repository at this point in the history
Can you do a fix PR for the global reference problem (semantic +
`globals`) to reduce the changes of this PR?

_Originally posted by @Boshen in
#8845 (review)
  • Loading branch information
shulaoda authored Feb 7, 2025
1 parent 515eb52 commit 44d985b
Show file tree
Hide file tree
Showing 20 changed files with 43 additions and 35 deletions.
11 changes: 10 additions & 1 deletion crates/oxc_linter/src/config/globals.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{borrow, fmt, hash};
use std::{borrow, fmt, hash, ops::Deref};

use rustc_hash::FxHashMap;
use schemars::JsonSchema;
Expand Down Expand Up @@ -32,6 +32,15 @@ use serde::{de::Visitor, Deserialize, Serialize};
// <https://eslint.org/docs/v8.x/use/configure/language-options#using-configuration-files-1>
#[derive(Debug, Default, Deserialize, Serialize, JsonSchema, Clone)]
pub struct OxlintGlobals(FxHashMap<String, GlobalValue>);

impl Deref for OxlintGlobals {
type Target = FxHashMap<String, GlobalValue>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl OxlintGlobals {
pub fn is_enabled<Q>(&self, name: &Q) -> bool
where
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub use config_builder::{ConfigBuilderError, ConfigStoreBuilder};
pub use config_store::ConfigStore;
pub(crate) use config_store::ResolvedLinterState;
pub use env::OxlintEnv;
pub use globals::OxlintGlobals;
pub use globals::{GlobalValue, OxlintGlobals};
pub use overrides::OxlintOverrides;
pub use oxlintrc::Oxlintrc;
pub use plugins::LintPlugins;
Expand Down
9 changes: 9 additions & 0 deletions crates/oxc_linter/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use std::{ops::Deref, path::Path, rc::Rc};

use oxc_ast::ast::IdentifierReference;
use oxc_cfg::ControlFlowGraph;
use oxc_diagnostics::{OxcDiagnostic, Severity};
use oxc_semantic::Semantic;
Expand All @@ -10,6 +11,7 @@ use oxc_span::{GetSpan, Span};
#[cfg(debug_assertions)]
use crate::rule::RuleFixMeta;
use crate::{
config::GlobalValue,
disable_directives::DisableDirectives,
fixer::{FixKind, Message, RuleFix, RuleFixer},
javascript_globals::GLOBALS,
Expand Down Expand Up @@ -148,6 +150,13 @@ impl<'a> LintContext<'a> {
&self.parent.config.globals
}

/// Checks if the provided identifier is a reference to a global variable.
pub fn is_reference_to_global_variable(&self, ident: &IdentifierReference) -> bool {
let name = ident.name.as_str();
self.scopes().root_unresolved_references().contains_key(name)
&& !self.globals().get(name).is_some_and(|value| *value == GlobalValue::Off)
}

/// Runtime environments turned on/off by the user.
///
/// Examples of environments are `builtin`, `browser`, `node`, etc.
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_alert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ fn is_global_this_ref_or_global_window<'a>(
return false;
};

if ctx.semantic().is_reference_to_global_variable(ident)
if ctx.is_reference_to_global_variable(ident)
&& (expr.is_specific_id(GLOBAL_WINDOW) || (expr.is_specific_id(GLOBAL_THIS)))
{
return !is_shadowed(scope_id, ident.name.as_str(), ctx);
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl Rule for NoConsole {
return;
};

if ctx.semantic().is_reference_to_global_variable(ident)
if ctx.is_reference_to_global_variable(ident)
&& ident.name == "console"
&& !self.allow.iter().any(|s| mem.static_property_name().is_some_and(|f| f == s))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl NoConstantBinaryExpression {
Expression::CallExpression(call_expr) => {
if let Expression::Identifier(ident) = &call_expr.callee {
return ["Boolean", "String", "Number"].contains(&ident.name.as_str())
&& ctx.semantic().is_reference_to_global_variable(ident);
&& ctx.is_reference_to_global_variable(ident);
}
false
}
Expand Down Expand Up @@ -292,15 +292,12 @@ impl NoConstantBinaryExpression {
Expression::CallExpression(call_expr) => {
if let Expression::Identifier(ident) = &call_expr.callee {
if ident.name == "String"
|| ident.name == "Number"
&& ctx.semantic().is_reference_to_global_variable(ident)
|| ident.name == "Number" && ctx.is_reference_to_global_variable(ident)
{
return true;
}

if ident.name == "Boolean"
&& ctx.semantic().is_reference_to_global_variable(ident)
{
if ident.name == "Boolean" && ctx.is_reference_to_global_variable(ident) {
return call_expr
.arguments
.iter()
Expand Down Expand Up @@ -342,7 +339,7 @@ impl NoConstantBinaryExpression {
Expression::NewExpression(call_expr) => {
if let Expression::Identifier(ident) = &call_expr.callee {
return ctx.env_contains_var(ident.name.as_str())
&& ctx.semantic().is_reference_to_global_variable(ident);
&& ctx.is_reference_to_global_variable(ident);
}
false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl Rule for NoNewNativeNonconstructor {
return;
};
if matches!(ident.name.as_str(), "Symbol" | "BigInt")
&& ctx.semantic().is_reference_to_global_variable(ident)
&& ctx.is_reference_to_global_variable(ident)
{
let start = expr.span.start;
let end = start + 3;
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_new_wrappers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl Rule for NoNewWrappers {
return;
};
if (ident.name == "String" || ident.name == "Number" || ident.name == "Boolean")
&& ctx.semantic().is_reference_to_global_variable(ident)
&& ctx.is_reference_to_global_variable(ident)
{
ctx.diagnostic(no_new_wrappers_diagnostic(ident.name.as_str(), expr.span));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_obj_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ fn resolve_global_binding<'a, 'b: 'a>(
let nodes = ctx.nodes();
let symbols = ctx.symbols();

if ctx.semantic().is_reference_to_global_variable(ident) {
if ctx.is_reference_to_global_variable(ident) {
return Some(ident.name.as_str());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl Rule for NoRestrictedGlobals {
return;
};

if ctx.semantic().is_reference_to_global_variable(ident) {
if ctx.is_reference_to_global_variable(ident) {
ctx.diagnostic(no_restricted_globals(&ident.name, message, ident.span));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ impl Rule for PreferExponentiationOperator {

match member_expor_obj {
Expression::Identifier(ident) => {
if ident.name.as_str() == "Math"
&& ctx.semantic().is_reference_to_global_variable(ident)
{
if ident.name.as_str() == "Math" && ctx.is_reference_to_global_variable(ident) {
ctx.diagnostic(prefer_exponentian_operator_diagnostic(call_expr.span));
}
}
Expand All @@ -70,7 +68,7 @@ impl Rule for PreferExponentiationOperator {

if let Expression::Identifier(ident) = member_expr.object().without_parentheses() {
if GLOBAL_OBJECT_NAMES.contains(ident.name.as_str())
&& ctx.semantic().is_reference_to_global_variable(ident)
&& ctx.is_reference_to_global_variable(ident)
{
ctx.diagnostic(prefer_exponentian_operator_diagnostic(call_expr.span));
}
Expand Down
7 changes: 2 additions & 5 deletions crates/oxc_linter/src/rules/eslint/prefer_object_spread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,13 @@ impl Rule for PreferObjectSpread {

match callee.object().get_inner_expression() {
Expression::Identifier(ident) => {
if ident.name != "Object" || !ctx.semantic().is_reference_to_global_variable(ident)
{
if ident.name != "Object" || !ctx.is_reference_to_global_variable(ident) {
return;
}
}
Expression::StaticMemberExpression(member_expr) => {
if let Expression::Identifier(ident) = member_expr.object.get_inner_expression() {
if ident.name != "globalThis"
|| !ctx.semantic().is_reference_to_global_variable(ident)
{
if ident.name != "globalThis" || !ctx.is_reference_to_global_variable(ident) {
return;
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/symbol_description.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl Rule for SymbolDescription {

if ident.name == "Symbol"
&& call_expr.arguments.len() == 0
&& ctx.semantic().is_reference_to_global_variable(ident)
&& ctx.is_reference_to_global_variable(ident)
{
ctx.diagnostic(symbol_description_diagnostic(call_expr.span));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/valid_typeof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl Rule for ValidTypeof {
}

if let Expression::Identifier(ident) = sibling {
if ident.name == "undefined" && ctx.semantic().is_reference_to_global_variable(ident) {
if ident.name == "undefined" && ctx.is_reference_to_global_variable(ident) {
ctx.diagnostic_with_fix(
if self.require_string_literals {
not_string(
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_linter/src/rules/jest/no_disabled_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,7 @@ fn run<'a>(possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>)
ctx.diagnostic(no_disabled_tests_diagnostic(error, help, call_expr.callee.span()));
}
} else if let Expression::Identifier(ident) = &call_expr.callee {
if ident.name.as_str() == "pending"
&& ctx.semantic().is_reference_to_global_variable(ident)
{
if ident.name.as_str() == "pending" && ctx.is_reference_to_global_variable(ident) {
// `describe('foo', function () { pending() })`
let (error, help) = Message::Pending.details();
ctx.diagnostic(no_disabled_tests_diagnostic(error, help, call_expr.span));
Expand Down Expand Up @@ -268,7 +266,7 @@ fn test() {
r#"it("contains a call to pending", function () { pending() })"#,
"pending();",
r#"
import { describe } from 'vitest';
import { describe } from 'vitest';
describe.skip("foo", function () {})
"#,
];
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/promise/avoid_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl Rule for AvoidNew {
return;
};

if ident.name == "Promise" && ctx.semantic().is_reference_to_global_variable(ident) {
if ident.name == "Promise" && ctx.is_reference_to_global_variable(ident) {
ctx.diagnostic(avoid_new_promise_diagnostic(expr.span));
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/promise/no_new_statics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Rule for NoNewStatics {
return;
};

if ident.name != "Promise" || !ctx.semantic().is_reference_to_global_variable(ident) {
if ident.name != "Promise" || !ctx.is_reference_to_global_variable(ident) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/react/exhaustive_deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ impl Rule for ExhaustiveDeps {
Expression::ArrayExpression(array_expr) => Some(array_expr),
Expression::Identifier(ident)
if ident.name == "undefined"
&& ctx.semantic().is_reference_to_global_variable(ident) =>
&& ctx.is_reference_to_global_variable(ident) =>
{
None
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/unicorn/new_for_builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ fn is_expr_global_builtin<'a, 'b>(
) -> Option<&'b str> {
match expr {
Expression::Identifier(ident) => {
if !ctx.semantic().is_reference_to_global_variable(ident) {
if !ctx.is_reference_to_global_variable(ident) {
return None;
}
Some(ident.name.as_str())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ source: crates/oxc_linter/src/tester.rs

eslint-plugin-jest(no-disabled-tests): Disabled test suite
╭─[no_disabled_tests.tsx:3:13]
2import { describe } from 'vitest';
2import { describe } from 'vitest';
3describe.skip("foo", function () {})
· ─────────────
4
Expand Down

0 comments on commit 44d985b

Please sign in to comment.