Skip to content

Commit

Permalink
fix(analyzer): False positive in rule noUndeclaredVariables #243
Browse files Browse the repository at this point in the history
  • Loading branch information
denbezrukov committed Sep 20, 2023
1 parent 55f740c commit 8acf9be
Show file tree
Hide file tree
Showing 13 changed files with 180 additions and 11 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
### JavaScript APIs
### Linter

#### Bug fixes

- Fix [#243](https://github.com/biomejs/biome/issues/243) a false positive case where the incorrect scope was defined for the `infer` type. in rule [noUndeclaredVariables](https://biomejs.dev/linter/rules/no-undeclared-variables/). Contributed by @denbezrukov

#### New features

- Add [noMisleadingInstantiator](https://biomejs.dev/linter/rules/no-mileading-instantiator) rule. The rule reports the misleading use of the `new` and `constructor` methods. Contributed by @unvalley
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export type WithSelectors<S> = S extends { getState: () => infer T }
? S & { use: { [K in keyof T]: () => T[K] } }
: never;

type A = number extends infer T ? T : never;
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: infer.ts
---
# Input
```js
export type WithSelectors<S> = S extends { getState: () => infer T }
? S & { use: { [K in keyof T]: () => T[K] } }
: never;

type A = number extends infer T ? T : never;

```


Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
type A = number extends infer T ? never : T;
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: infer_incorrect.ts
---
# Input
```js
type A = number extends infer T ? never : T;

```

# Diagnostics
```
infer_incorrect.ts:1:43 lint/correctness/noUndeclaredVariables ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The T variable is undeclared
> 1 │ type A = number extends infer T ? never : T;
│ ^
2 │
```


64 changes: 59 additions & 5 deletions crates/biome_js_semantic/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
use rustc_hash::FxHashMap;
use std::collections::{HashMap, VecDeque};
use std::mem;

use biome_js_syntax::AnyTsType;
use biome_js_syntax::{
AnyJsAssignment, AnyJsAssignmentPattern, AnyJsExpression, JsAssignmentExpression,
JsCallExpression, JsForVariableDeclaration, JsIdentifierAssignment, JsIdentifierBinding,
Expand Down Expand Up @@ -162,6 +164,7 @@ pub struct SemanticEventExtractor {
/// At any point this is the set of available bindings and
/// the range of its declaration
bindings: FxHashMap<TokenText, BindingInfo>,
infers: Vec<TsTypeParameterName>, //pending bindings?
}

/// Holds the text range of the token when it is bound,
Expand Down Expand Up @@ -247,6 +250,7 @@ impl SemanticEventExtractor {
scopes: vec![],
next_scope_id: 0,
bindings: FxHashMap::default(),
infers: vec![],
}
}

Expand Down Expand Up @@ -304,7 +308,6 @@ impl SemanticEventExtractor {
| TS_INTERFACE_DECLARATION
| TS_ENUM_DECLARATION
| TS_TYPE_ALIAS_DECLARATION
| TS_FUNCTION_TYPE
| TS_DECLARE_FUNCTION_DECLARATION => {
self.push_scope(
node.text_range(),
Expand All @@ -321,8 +324,11 @@ impl SemanticEventExtractor {
false,
);
}

_ => {}
_ => {
if let Some(node) = node.clone().cast::<AnyTsType>() {
self.enter_any_type(&node);
}
}
}
}

Expand All @@ -348,6 +354,18 @@ impl SemanticEventExtractor {
Some(is_var)
}

fn enter_any_type(&mut self, node: &AnyTsType) {
if node.in_conditional_true_type() {
self.push_conditional_true_scope(node);
} else if let Some(node) = node.as_ts_function_type() {
self.push_scope(
node.syntax().text_range(),
ScopeHoisting::DontHoistDeclarationsToParent,
false,
);
}
}

fn enter_identifier_binding(&mut self, node: &JsSyntaxNode) -> Option<()> {
use JsSyntaxKind::*;
debug_assert!(matches!(
Expand All @@ -371,6 +389,13 @@ impl SemanticEventExtractor {
TS_TYPE_PARAMETER_NAME => {
let token = node.clone().cast::<TsTypeParameterName>()?;
let name_token = token.ident_token().ok()?;

if token.in_infer_type() {
self.infers.push(token);

return None;
}

let is_var = Some(false);
(name_token, is_var)
}
Expand Down Expand Up @@ -562,15 +587,26 @@ impl SemanticEventExtractor {
| JS_CATCH_CLAUSE
| JS_STATIC_INITIALIZATION_BLOCK_CLASS_MEMBER
| TS_DECLARE_FUNCTION_DECLARATION
| TS_FUNCTION_TYPE
| TS_INTERFACE_DECLARATION
| TS_ENUM_DECLARATION
| TS_TYPE_ALIAS_DECLARATION
| TS_MODULE_DECLARATION
| TS_EXTERNAL_MODULE_DECLARATION => {
self.pop_scope(node.text_range());
}
_ => {}
_ => {
if let Some(node) = node.clone().cast::<AnyTsType>() {
self.leave_any_type(&node);
}
}
}
}

fn leave_any_type(&mut self, node: &AnyTsType) {
if node.in_conditional_true_type() {
self.pop_scope(node.syntax().text_range());
} else if let Some(node) = node.as_ts_function_type() {
self.pop_scope(node.syntax().text_range());
}
}

Expand All @@ -580,6 +616,24 @@ impl SemanticEventExtractor {
self.stash.pop_front()
}

fn push_conditional_true_scope(&mut self, node: &AnyTsType) {
self.push_scope(
node.syntax().text_range(),
ScopeHoisting::DontHoistDeclarationsToParent,
false,
);

let infers = mem::take(&mut self.infers);
for infer in infers {
let name_token = infer.ident_token().ok();
let parent_kind = infer.syntax().parent().map(|parent| parent.kind());

if let (Some(name_token), Some(parent_kind)) = (name_token, parent_kind) {
self.push_binding_into_scope(None, &name_token, &parent_kind);
}
}
}

fn push_scope(&mut self, range: TextRange, hoisting: ScopeHoisting, is_closure: bool) {
let scope_id = self.next_scope_id;
self.next_scope_id += 1;
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_js_semantic/src/semantic_model/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub(crate) struct SemanticModelScopeData {
pub(crate) children: Vec<usize>,
// All bindings of this scope (points to SemanticModelData::bindings)
pub(crate) bindings: Vec<usize>,
// Map pointing to the [bindings] vec of each bindings by its name
// Map pointing to the [bindings] vec of each bindings by its name
pub(crate) bindings_by_name: FxHashMap<TokenText, usize>,
// All read references of a scope
pub(crate) read_references: Vec<SemanticModelScopeReference>,
Expand Down
8 changes: 8 additions & 0 deletions crates/biome_js_semantic/src/tests/infer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
use crate::assert_semantics;

assert_semantics! {
ok_conditional_true_type_scope, "type A<T> = T extends string ? (/*START Scope*/number)/*END Scope*/: boolean;",
ok_conditional_true_type_infer_simple, "type A<T> = T extends infer /*@ Scope */T ? (/*START Scope*/number)/*END Scope*/: boolean;",
ok_conditional_true_type_infer_function, "type A<T> = T extends { getState: () => infer /*@ Scope */ T } ? (/*START Scope*/number)/*END Scope*/: boolean;",
ok_conditional_true_type_infer_nested, "type A = MyType extends (OtherType extends infer /*@ InnerScope */T ? (/*START InnerScope*/infer /*@ OuterScope */ U)/*END InnerScope*/ : InnerFalse) ? (/*START OuterScope*/OuterTrue)/*END OuterScope*/ : OuterFalse;",
}
1 change: 1 addition & 0 deletions crates/biome_js_semantic/src/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
mod assertions;
pub mod declarations;
mod functions;
mod infer;
mod references;
mod scopes;

Expand Down
4 changes: 2 additions & 2 deletions crates/biome_js_semantic/src/tests/scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ assert_semantics! {
ok_scope_static_initialization_block,
"class A {
static/*START A*/ {
const a/*@ A*/ = 2;
}/*END A*/
const a/*@ A*/ = 2;
}/*END A*/
};",
}

Expand Down
57 changes: 56 additions & 1 deletion crates/biome_js_syntax/src/type_ext.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use biome_rowan::AstNode;
use std::iter;

use crate::AnyTsType;
use crate::{AnyTsType, TsConditionalType, TsInferType, TsTypeParameterName};

impl AnyTsType {
/// Try to extract non `TsParenthesizedType` from `AnyTsType`
Expand Down Expand Up @@ -77,4 +78,58 @@ impl AnyTsType {
| AnyTsType::TsStringType(_)
)
}

/// Checks if `self` stands as the `true_type` of a conditional type in Typescript.
///
/// # Examples
///
/// ```rust
/// use biome_js_factory::make;
/// use biome_js_syntax::T;
/// use biome_js_syntax::AnyTsType;
///
/// let check_type = AnyTsType::TsNumberType(make::ts_number_type(make::token(T![number])));
/// let extends_type = AnyTsType::TsNumberType(make::ts_number_type(make::token(T![number])));
/// let true_type = AnyTsType::TsNumberType(make::ts_number_type(make::token(T![number])));
/// let false_type = AnyTsType::TsNumberType(make::ts_number_type(make::token(T![number])));
///
/// let conditional = make::ts_conditional_type(
/// check_type,
/// make::token(T![extends]),
/// extends_type,
/// make::token(T![?]),
/// true_type,
/// make::token(T![:]),
/// false_type,
/// );
///
/// assert!(!conditional.check_type().unwrap().in_conditional_true_type());
/// assert!(!conditional.extends_type().unwrap().in_conditional_true_type());
/// assert!(conditional.true_type().unwrap().in_conditional_true_type());
/// assert!(!conditional.false_type().unwrap().in_conditional_true_type());
/// ```
pub fn in_conditional_true_type(&self) -> bool {
self.parent::<TsConditionalType>()
.and_then(|parent| parent.true_type().ok())
.map_or(false, |ref true_type| true_type == self)
}
}

impl TsTypeParameterName {
/// Checks if `self` is the type being inferred in a TypeScript `TsInferType`.
///
/// # Examples
///
/// ```rust
/// use biome_js_factory::make;
/// use biome_js_syntax::T;
///
/// let infer = make::ts_infer_type(make::token(T![infer]), make::ts_type_parameter_name(make::ident("T"))).build();
/// assert!(infer.name().unwrap().in_infer_type());
/// ```
pub fn in_infer_type(&self) -> bool {
self.parent::<TsInferType>()
.and_then(|parent| parent.name().ok())
.map_or(false, |ref name| name == self)
}
}
2 changes: 0 additions & 2 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions website/src/content/docs/internals/changelog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
### JavaScript APIs
### Linter

#### Bug fixes

- Fix [#243](https://github.com/biomejs/biome/issues/243) a false positive case where the incorrect scope was defined for the `infer` type. in rule [noUndeclaredVariables](https://biomejs.dev/linter/rules/no-undeclared-variables/). Contributed by @denbezrukov

#### New features

- Add [noMisleadingInstantiator](https://biomejs.dev/linter/rules/no-mileading-instantiator) rule. The rule reports the misleading use of the `new` and `constructor` methods. Contributed by @unvalley
Expand Down

0 comments on commit 8acf9be

Please sign in to comment.