From 3df5f4360842276b38fee1a5978efe6fc8417d26 Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Mon, 9 Oct 2023 22:02:18 +0200 Subject: [PATCH] fix(js_semantic): type value schism (#495) --- CHANGELOG.md | 2 + .../correctness/no_unused_variables.rs | 3 + .../nursery/use_exhaustive_dependencies.rs | 3 +- .../style/use_naming_convention.rs | 3 +- .../invalidTypeValueWithSameName.ts | 2 + .../invalidTypeValueWithSameName.ts.snap | 24 + .../invalidTypeValueSameNames.ts | 10 + .../invalidTypeValueSameNames.ts.snap | 79 ++ .../correctness/noUnusedVariables/issue104.ts | 6 + .../noUnusedVariables/issue104.ts.snap | 16 + .../noUnusedVariables/validValueExportType.ts | 5 + .../validValueExportType.ts.snap | 14 + crates/biome_js_semantic/src/events.rs | 999 ++++++++---------- .../src/semantic_model/builder.rs | 12 +- .../src/semantic_model/tests.rs | 57 +- .../biome_js_semantic/src/tests/assertions.rs | 35 +- crates/biome_js_semantic/src/tests/scopes.rs | 2 +- crates/biome_js_syntax/src/binding_ext.rs | 40 +- crates/biome_js_syntax/src/export_ext.rs | 32 + crates/biome_js_syntax/src/import_ext.rs | 36 +- crates/biome_js_syntax/src/lib.rs | 2 + crates/biome_js_syntax/src/stmt_ext.rs | 17 +- crates/biome_js_syntax/src/type_ext.rs | 21 +- crates/biome_rowan/src/macros.rs | 4 +- .../src/content/docs/internals/changelog.mdx | 2 + xtask/coverage/src/symbols/msts.rs | 29 +- 26 files changed, 770 insertions(+), 685 deletions(-) create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalidTypeValueWithSameName.ts create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalidTypeValueWithSameName.ts.snap create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidTypeValueSameNames.ts create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidTypeValueSameNames.ts.snap create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/issue104.ts create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/issue104.ts.snap create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validValueExportType.ts create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validValueExportType.ts.snap create mode 100644 crates/biome_js_syntax/src/export_ext.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f04c264c9db..598ee551d4ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [452](https://github.com/biomejs/biome/pull/452). The linter panicked when it met a malformed regex (a regex not ending with a slash). +- Fix [#104](https://github.com/biomejs/biome/issues/104). We now correctly handle types and values with the same name. + ### Parser - Enhance diagnostic for infer type handling in the parser. The 'infer' keyword can only be utilized within the 'extends' clause of a conditional type. Using it outside of this context will result in an error. Ensure that any type declarations using 'infer' are correctly placed within the conditional type structure to avoid parsing issues. Contributed by @denbezrukov diff --git a/crates/biome_js_analyze/src/semantic_analyzers/correctness/no_unused_variables.rs b/crates/biome_js_analyze/src/semantic_analyzers/correctness/no_unused_variables.rs index b52e9849ad58..ca6eb6000bbe 100644 --- a/crates/biome_js_analyze/src/semantic_analyzers/correctness/no_unused_variables.rs +++ b/crates/biome_js_analyze/src/semantic_analyzers/correctness/no_unused_variables.rs @@ -153,6 +153,9 @@ fn suggested_fix_if_unused(binding: &AnyJsIdentifierBinding) -> Option None, // Some parameters are ok to not be used + AnyJsBindingDeclaration::JsArrowFunctionExpression(_) => { + suggestion_for_binding(binding) + } AnyJsBindingDeclaration::TsPropertyParameter(_) => None, AnyJsBindingDeclaration::JsFormalParameter(parameter) => { if is_function_that_is_ok_parameter_not_be_used(¶meter.parent_function()) { diff --git a/crates/biome_js_analyze/src/semantic_analyzers/nursery/use_exhaustive_dependencies.rs b/crates/biome_js_analyze/src/semantic_analyzers/nursery/use_exhaustive_dependencies.rs index ad7b663d5b5e..4a3963f558ff 100644 --- a/crates/biome_js_analyze/src/semantic_analyzers/nursery/use_exhaustive_dependencies.rs +++ b/crates/biome_js_analyze/src/semantic_analyzers/nursery/use_exhaustive_dependencies.rs @@ -469,7 +469,8 @@ fn capture_needs_to_be_in_the_dependency_list( } // all others need to be in the dependency list - AnyJsBindingDeclaration::JsFormalParameter(_) + AnyJsBindingDeclaration::JsArrowFunctionExpression(_) + | AnyJsBindingDeclaration::JsFormalParameter(_) | AnyJsBindingDeclaration::JsRestParameter(_) | AnyJsBindingDeclaration::JsBogusParameter(_) | AnyJsBindingDeclaration::TsIndexSignatureParameter(_) diff --git a/crates/biome_js_analyze/src/semantic_analyzers/style/use_naming_convention.rs b/crates/biome_js_analyze/src/semantic_analyzers/style/use_naming_convention.rs index 1433f682ddcf..1d396f843678 100644 --- a/crates/biome_js_analyze/src/semantic_analyzers/style/use_naming_convention.rs +++ b/crates/biome_js_analyze/src/semantic_analyzers/style/use_naming_convention.rs @@ -791,7 +791,8 @@ impl Named { AnyJsBindingDeclaration::JsVariableDeclarator(var) => { Named::from_variable_declarator(var) } - AnyJsBindingDeclaration::JsBogusParameter(_) + AnyJsBindingDeclaration::JsArrowFunctionExpression(_) + | AnyJsBindingDeclaration::JsBogusParameter(_) | AnyJsBindingDeclaration::JsFormalParameter(_) | AnyJsBindingDeclaration::JsRestParameter(_) => Some(Named::FunctionParameter), AnyJsBindingDeclaration::JsCatchDeclaration(_) => Some(Named::CatchParameter), diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalidTypeValueWithSameName.ts b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalidTypeValueWithSameName.ts new file mode 100644 index 000000000000..218d769751bf --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalidTypeValueWithSameName.ts @@ -0,0 +1,2 @@ +const a = 0; +export type { a } \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalidTypeValueWithSameName.ts.snap b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalidTypeValueWithSameName.ts.snap new file mode 100644 index 000000000000..e79380ffdd63 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalidTypeValueWithSameName.ts.snap @@ -0,0 +1,24 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: invalidTypeValueWithSameName.ts +--- +# Input +```js +const a = 0; +export type { a } +``` + +# Diagnostics +``` +invalidTypeValueWithSameName.ts:2:15 lint/correctness/noUndeclaredVariables ━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The a variable is undeclared + + 1 │ const a = 0; + > 2 │ export type { a } + │ ^ + + +``` + + diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidTypeValueSameNames.ts b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidTypeValueSameNames.ts new file mode 100644 index 000000000000..3c6d2e14ddcc --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidTypeValueSameNames.ts @@ -0,0 +1,10 @@ + +type a = number +export const a = 5; + +function f() {} +export type f = () => {} + +const b = true +type b = boolean +export { type b } diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidTypeValueSameNames.ts.snap b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidTypeValueSameNames.ts.snap new file mode 100644 index 000000000000..675ee3320dd9 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidTypeValueSameNames.ts.snap @@ -0,0 +1,79 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: invalidTypeValueSameNames.ts +--- +# Input +```js + +type a = number +export const a = 5; + +function f() {} +export type f = () => {} + +const b = true +type b = boolean +export { type b } + +``` + +# Diagnostics +``` +invalidTypeValueSameNames.ts:2:6 lint/correctness/noUnusedVariables ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This type alias is unused. + + > 2 │ type a = number + │ ^ + 3 │ export const a = 5; + 4 │ + + i Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + +``` +invalidTypeValueSameNames.ts:5:10 lint/correctness/noUnusedVariables ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This function is unused. + + 3 │ export const a = 5; + 4 │ + > 5 │ function f() {} + │ ^ + 6 │ export type f = () => {} + 7 │ + + i Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + +``` +invalidTypeValueSameNames.ts:8:7 lint/correctness/noUnusedVariables FIXABLE ━━━━━━━━━━━━━━━━━━━━━━ + + ! This variable is unused. + + 6 │ export type f = () => {} + 7 │ + > 8 │ const b = true + │ ^ + 9 │ type b = boolean + 10 │ export { type b } + + i Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + i Unsafe fix: If this is intentional, prepend b with an underscore. + + 6 6 │ export type f = () => {} + 7 7 │ + 8 │ - const·b·=·true + 8 │ + const·_b·=·true + 9 9 │ type b = boolean + 10 10 │ export { type b } + + +``` + + diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/issue104.ts b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/issue104.ts new file mode 100644 index 000000000000..1fee59b71e79 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/issue104.ts @@ -0,0 +1,6 @@ +// See https://github.com/biomejs/biome/issues/104 + +import { X } from "mod" +export function f(X: X): X { + return X; +} diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/issue104.ts.snap b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/issue104.ts.snap new file mode 100644 index 000000000000..576197d8c3d1 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/issue104.ts.snap @@ -0,0 +1,16 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: issue104.ts +--- +# Input +```js +// See https://github.com/biomejs/biome/issues/104 + +import { X } from "mod" +export function f(X: X): X { + return X; +} + +``` + + diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validValueExportType.ts b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validValueExportType.ts new file mode 100644 index 000000000000..dbbf9778e08c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validValueExportType.ts @@ -0,0 +1,5 @@ +class C {} +enum E {} + +export type { C } +export { type E } \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validValueExportType.ts.snap b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validValueExportType.ts.snap new file mode 100644 index 000000000000..3895ef70c925 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/validValueExportType.ts.snap @@ -0,0 +1,14 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: validValueExportType.ts +--- +# Input +```js +class C {} +enum E {} + +export type { C } +export { type E } +``` + + diff --git a/crates/biome_js_semantic/src/events.rs b/crates/biome_js_semantic/src/events.rs index c1ed5311c158..8bece7a44c70 100644 --- a/crates/biome_js_semantic/src/events.rs +++ b/crates/biome_js_semantic/src/events.rs @@ -1,38 +1,36 @@ //! Events emitted by the [SemanticEventExtractor] which are then constructed into the Semantic Model -use biome_js_syntax::binding_ext::AnyJsIdentifierBinding; -use rustc_hash::FxHashMap; -use std::collections::{HashMap, VecDeque}; -use std::mem; - -use biome_js_syntax::AnyTsType; +use biome_js_syntax::binding_ext::{AnyJsBindingDeclaration, AnyJsIdentifierBinding}; use biome_js_syntax::{ - AnyJsAssignment, AnyJsAssignmentPattern, JsAssignmentExpression, JsForVariableDeclaration, - JsIdentifierAssignment, JsLanguage, JsReferenceIdentifier, JsSyntaxKind, JsSyntaxNode, - JsSyntaxToken, JsVariableDeclaration, JsxReferenceIdentifier, TextRange, TextSize, + AnyJsExportNamedSpecifier, AnyJsNamedImportSpecifier, AnyTsType, JsImportNamedClause, +}; +use biome_js_syntax::{ + AnyJsIdentifierUsage, JsLanguage, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, TextRange, TsTypeParameterName, }; use biome_rowan::{syntax::Preorder, AstNode, SyntaxNodeOptionExt, TokenText}; +use rustc_hash::FxHashMap; +use std::collections::VecDeque; +use std::mem; +use JsSyntaxKind::*; -/// Events emitted by the [SemanticEventExtractor]. These events are later -/// made into the Semantic Model. -#[derive(Debug)] +/// Events emitted by the [SemanticEventExtractor]. +/// These events are later made into the Semantic Model. +#[derive(Debug, Eq, PartialEq)] pub enum SemanticEvent { /// Tracks where a new symbol declaration is found. /// Generated for: /// - Variable Declarations /// - Import bindings /// - Functions parameters + /// - Type parameters DeclarationFound { - range: TextRange, - scope_started_at: TextSize, + name_token: JsSyntaxToken, scope_id: usize, hoisted_scope_id: Option, - name: TokenText, }, - /// Tracks where a symbol is read, but only if its declaration - /// is before this reference. + /// Tracks where a symbol is read, but only if its declaration is before this reference. /// Generated for: /// - All reference identifiers Read { @@ -41,8 +39,8 @@ pub enum SemanticEvent { scope_id: usize, }, - /// Tracks where a symbol is read, but only if its declaration - /// was hoisted. This means that its declaration is after this reference. + /// Tracks where a symbol is read, but only if its declaration was hoisted. + /// This means that its declaration is after this reference. /// - All reference identifiers HoistedRead { range: TextRange, @@ -50,8 +48,7 @@ pub enum SemanticEvent { scope_id: usize, }, - /// Tracks where a symbol is written, but only if its declaration - /// is before this reference. + /// Tracks where a symbol is written, but only if its declaration is before this reference. /// Generated for: /// - All identifier assignments Write { @@ -60,8 +57,8 @@ pub enum SemanticEvent { scope_id: usize, }, - /// Tracks where a symbol is written, but only if its declaration - /// was hoisted. This means that its declaration is after this reference. + /// Tracks where a symbol is written, but only if its declaration was hoisted. + /// This means that its declaration is after this reference. /// Generated for: /// - All identifier assignments HoistedWrite { @@ -80,6 +77,7 @@ pub enum SemanticEvent { /// - Blocks /// - Function body ScopeStarted { + /// Scope range range: TextRange, scope_id: usize, parent_scope_id: Option, @@ -91,38 +89,30 @@ pub enum SemanticEvent { /// - Blocks /// - Function body ScopeEnded { + /// Scope range range: TextRange, - started_at: TextSize, scope_id: usize, }, /// Tracks where a symbol is exported. - /// The range points to the binding that - /// is being exported + /// The range points to the binding that is being exported. Exported { range: TextRange }, } impl SemanticEvent { - pub fn range(&self) -> &TextRange { + pub fn range(&self) -> TextRange { match self { - SemanticEvent::DeclarationFound { range, .. } - | SemanticEvent::ScopeStarted { range, .. } - | SemanticEvent::ScopeEnded { range, .. } - | SemanticEvent::Read { range, .. } - | SemanticEvent::HoistedRead { range, .. } - | SemanticEvent::Write { range, .. } - | SemanticEvent::HoistedWrite { range, .. } - | SemanticEvent::UnresolvedReference { range, .. } - | SemanticEvent::Exported { range } => range, + Self::DeclarationFound { name_token, .. } => name_token.text_range(), + Self::ScopeStarted { range, .. } + | Self::ScopeEnded { range, .. } + | Self::Read { range, .. } + | Self::HoistedRead { range, .. } + | Self::Write { range, .. } + | Self::HoistedWrite { range, .. } + | Self::UnresolvedReference { range, .. } + | Self::Exported { range } => *range, } } - - pub fn str<'a>(&self, code: &'a str) -> &'a str { - let range = self.range(); - let start: u32 = range.start().into(); - let end: u32 = range.end().into(); - &code[start as usize..end as usize] - } } /// Extracts [SemanticEvent] from [JsSyntaxNode]. @@ -158,50 +148,82 @@ impl SemanticEvent { /// ``` #[derive(Default, Debug)] pub struct SemanticEventExtractor { + /// Event queue stash: VecDeque, + /// Stack of scopes scopes: Vec, - next_scope_id: usize, - /// At any point this is the set of available bindings and - /// the range of its declaration - bindings: FxHashMap, + /// Number of generated scopes + /// This allows assigning a unique scope id to every scope. + scope_count: usize, + /// At any point this is the set of available bindings and their range in the current scope + bindings: FxHashMap, + /// Type parameters bound in a `infer T` clause. infers: Vec, } -/// Holds the text range of the token when it is bound, -/// along with the kind of declaration -#[derive(Debug)] -struct BindingInfo { - text_range: TextRange, - /// For export determination, it is necessary to provide - /// information on how a specific token is bound - declaration_kind: JsSyntaxKind, -} - +/// A binding name is either a type or a value. +/// +/// Two bindings (a type and a value bindings) can be associated to the same range. +/// This represents a declaration that is both a type and a value. +/// For example, in TypeScript a class and a enum are both a type and a value. +/// Allocating two bindings allows to for properly detecting type and value shadowing in inner scopes. #[derive(Debug, Hash, Eq, PartialEq, Clone)] -struct BindingName { - name: TokenText, +enum BindingName { + Type(TokenText), + Value(TokenText), } -#[derive(Debug)] +/// This type allows reporting a reference and bind to a binding (if any) later. +/// The range is the range of the referenced binding. +#[derive(Debug, Clone)] enum Reference { - Read { range: TextRange, is_exported: bool }, - Write { range: TextRange }, + /// Read and export a type, a value, or both. + /// ```js + /// export { A } + /// ``` + Export(TextRange), + /// Read and export only a type + /// ```ts + /// export { type T1 } + /// export type { T2, T3 } + /// ``` + ExportType(TextRange), + /// All reads that are not an export + /// ```js + /// f(); + /// a; + /// ``` + Read(TextRange), + /// Assignment + /// ```js + /// a = 0; + /// a += 1; + /// ``` + Write(TextRange), } impl Reference { - fn is_read(&self) -> bool { - matches!(self, Reference::Read { .. }) + const fn is_write(&self) -> bool { + matches!(self, Self::Write { .. }) + } + + const fn is_export(&self) -> bool { + matches!(self, Self::Export { .. }) } - pub fn range(&self) -> &TextRange { + /// Range of the referenced binding + const fn range(&self) -> &TextRange { match self { - Reference::Read { range, .. } | Reference::Write { range } => range, + Self::Export(range) + | Self::ExportType(range) + | Self::Read(range) + | Self::Write(range) => range, } } } -#[derive(Debug)] -pub enum ScopeHoisting { +#[derive(Debug, Eq, PartialEq)] +enum ScopeHoisting { DontHoistDeclarationsToParent, HoistDeclarationsToParent, } @@ -209,17 +231,13 @@ pub enum ScopeHoisting { #[derive(Debug)] struct Scope { scope_id: usize, - started_at: TextSize, /// All bindings declared inside this scope bindings: Vec, - /// References that still needs to be bound and - /// will be solved at the end of the scope - references: HashMap>, - /// All bindings that where shadowed and will be - /// restored after this scope ends. - shadowed: Vec<(BindingName, BindingInfo)>, - /// If this scope allows declarations to be hoisted - /// to parent scope or not + /// References that still needs to be bound and will be solved at the end of the scope + references: FxHashMap>, + /// All bindings that where shadowed and will be restored after this scope ends. + shadowed: Vec<(BindingName, TextRange)>, + /// If this scope allows declarations to be hoisted to parent scope or not hoisting: ScopeHoisting, } @@ -228,7 +246,7 @@ impl SemanticEventExtractor { Self { stash: VecDeque::new(), scopes: vec![], - next_scope_id: 0, + scope_count: 0, bindings: FxHashMap::default(), infers: vec![], } @@ -237,19 +255,13 @@ impl SemanticEventExtractor { /// See [SemanticEvent] for a more detailed description of which events [SyntaxNode] generates. #[inline] pub fn enter(&mut self, node: &JsSyntaxNode) { - use biome_js_syntax::JsSyntaxKind::*; - match node.kind() { JS_IDENTIFIER_BINDING | TS_IDENTIFIER_BINDING | TS_TYPE_PARAMETER_NAME => { self.enter_identifier_binding(&AnyJsIdentifierBinding::unwrap_cast(node.clone())); } - JS_REFERENCE_IDENTIFIER | JSX_REFERENCE_IDENTIFIER => { - self.enter_reference_identifier(node); - } - JS_IDENTIFIER_ASSIGNMENT => { - self.enter_js_identifier_assignment(&JsIdentifierAssignment::unwrap_cast( - node.clone(), - )); + + JS_REFERENCE_IDENTIFIER | JSX_REFERENCE_IDENTIFIER | JS_IDENTIFIER_ASSIGNMENT => { + self.enter_identifier_usage(AnyJsIdentifierUsage::unwrap_cast(node.clone())); } JS_MODULE | JS_SCRIPT => self.push_scope( @@ -286,7 +298,8 @@ impl SemanticEventExtractor { | TS_INTERFACE_DECLARATION | TS_ENUM_DECLARATION | TS_TYPE_ALIAS_DECLARATION - | TS_DECLARE_FUNCTION_DECLARATION => { + | TS_DECLARE_FUNCTION_DECLARATION + | TS_DECLARE_FUNCTION_EXPORT_DEFAULT_DECLARATION => { self.push_scope( node.text_range(), ScopeHoisting::DontHoistDeclarationsToParent, @@ -302,6 +315,7 @@ impl SemanticEventExtractor { false, ); } + _ => { if let Some(node) = AnyTsType::cast_ref(node) { self.enter_any_type(&node); @@ -310,23 +324,6 @@ impl SemanticEventExtractor { } } - fn is_var(declarator: &JsSyntaxNode) -> Option { - debug_assert!( - declarator.kind() == JsSyntaxKind::JS_VARIABLE_DECLARATOR, - "specified node is not a variable declarator (JS_VARIABLE_DECLARATOR)" - ); - let declarator_parent = declarator.parent()?; - Some(match declarator_parent.kind() { - JsSyntaxKind::JS_VARIABLE_DECLARATOR_LIST => { - JsVariableDeclaration::cast(declarator_parent.parent()?)?.is_var() - } - JsSyntaxKind::JS_FOR_VARIABLE_DECLARATION => { - JsForVariableDeclaration::unwrap_cast(declarator_parent).is_var() - } - _ => false, - }) - } - fn enter_any_type(&mut self, node: &AnyTsType) { if node.in_conditional_true_type() { self.push_conditional_true_scope(node); @@ -340,159 +337,218 @@ impl SemanticEventExtractor { } fn enter_identifier_binding(&mut self, node: &AnyJsIdentifierBinding) -> Option<()> { - use JsSyntaxKind::*; - - let name_token = match node { - AnyJsIdentifierBinding::JsIdentifierBinding(binding) => binding.name_token(), - AnyJsIdentifierBinding::TsIdentifierBinding(binding) => binding.name_token(), - AnyJsIdentifierBinding::TsTypeParameterName(binding) => { - if binding.in_infer_type() { - self.infers.push(binding.clone()); - return None; + let name_token = node.name_token().ok()?; + let name = name_token.token_text_trimmed(); + let name_range = name_token.text_range(); + let mut hoisted_scope_id = None; + let is_exported = if let Some(declaration) = node.declaration() { + let is_exported = declaration.export().is_some(); + match declaration { + AnyJsBindingDeclaration::JsVariableDeclarator(declarator) => { + hoisted_scope_id = if declarator.declaration()?.is_var() { + self.scope_index_to_hoist_declarations(0) + } else { + None + }; + self.push_binding(hoisted_scope_id, BindingName::Value(name), name_range); } - binding.ident_token() - } - }; - let name_token = name_token.ok()?; - - let node = node.syntax(); - let parent = node.parent()?; - let parent_kind = parent.kind(); - match parent_kind { - JS_VARIABLE_DECLARATOR => { - if let Some(true) = Self::is_var(&parent) { - let hoisted_scope_id = self.scope_index_to_hoist_declarations(0); - self.push_binding_into_scope(hoisted_scope_id, &name_token, parent_kind); - } else { - self.push_binding_into_scope(None, &name_token, parent_kind); - }; - self.export_variable_declarator(node, &parent); - } - JS_FUNCTION_DECLARATION | JS_FUNCTION_EXPORT_DEFAULT_DECLARATION => { - let hoisted_scope_id = self.scope_index_to_hoist_declarations(1); - self.push_binding_into_scope(hoisted_scope_id, &name_token, parent_kind); - self.export_function_declaration(node, &parent); - } - JS_CLASS_EXPRESSION | JS_FUNCTION_EXPRESSION => { - self.push_binding_into_scope(None, &name_token, parent_kind); - self.export_declaration_expression(node, &parent); - } - JS_CLASS_DECLARATION - | JS_CLASS_EXPORT_DEFAULT_DECLARATION - | TS_ENUM_DECLARATION - | TS_INTERFACE_DECLARATION - | TS_MODULE_DECLARATION - | TS_EXTERNAL_MODULE_DECLARATION - | TS_TYPE_ALIAS_DECLARATION => { - let parent_scope = self.scopes.get(self.scopes.len() - 2); - let parent_scope = parent_scope.map(|scope| scope.scope_id); - self.push_binding_into_scope(parent_scope, &name_token, parent_kind); - self.export_declaration(node, &parent); - } - JS_BINDING_PATTERN_WITH_DEFAULT - | JS_OBJECT_BINDING_PATTERN - | JS_OBJECT_BINDING_PATTERN_REST - | JS_OBJECT_BINDING_PATTERN_PROPERTY - | JS_OBJECT_BINDING_PATTERN_PROPERTY_LIST - | JS_OBJECT_BINDING_PATTERN_SHORTHAND_PROPERTY - | JS_ARRAY_BINDING_PATTERN - | JS_ARRAY_BINDING_PATTERN_ELEMENT_LIST - | JS_ARRAY_BINDING_PATTERN_REST_ELEMENT => { - self.push_binding_into_scope(None, &name_token, parent_kind); - - let possible_declarator = parent.ancestors().find(|x| { - !matches!( - x.kind(), - JS_BINDING_PATTERN_WITH_DEFAULT - | JS_OBJECT_BINDING_PATTERN - | JS_OBJECT_BINDING_PATTERN_REST - | JS_OBJECT_BINDING_PATTERN_PROPERTY - | JS_OBJECT_BINDING_PATTERN_PROPERTY_LIST - | JS_OBJECT_BINDING_PATTERN_SHORTHAND_PROPERTY - | JS_ARRAY_BINDING_PATTERN - | JS_ARRAY_BINDING_PATTERN_ELEMENT_LIST - | JS_ARRAY_BINDING_PATTERN_REST_ELEMENT - ) - })?; - - if let JS_VARIABLE_DECLARATOR = possible_declarator.kind() { - self.export_variable_declarator(node, &possible_declarator); + AnyJsBindingDeclaration::TsDeclareFunctionDeclaration(_) + | AnyJsBindingDeclaration::TsDeclareFunctionExportDefaultDeclaration(_) + | AnyJsBindingDeclaration::JsFunctionDeclaration(_) + | AnyJsBindingDeclaration::JsFunctionExportDefaultDeclaration(_) => { + hoisted_scope_id = self.scope_index_to_hoist_declarations(1); + self.push_binding(hoisted_scope_id, BindingName::Value(name), name_range); + } + AnyJsBindingDeclaration::JsClassExpression(_) + | AnyJsBindingDeclaration::JsFunctionExpression(_) => { + self.push_binding(None, BindingName::Value(name.clone()), name_range); + self.push_binding(None, BindingName::Type(name), name_range); + } + AnyJsBindingDeclaration::JsClassDeclaration(_) + | AnyJsBindingDeclaration::JsClassExportDefaultDeclaration(_) + | AnyJsBindingDeclaration::TsEnumDeclaration(_) => { + // These declarations have their own scope. + // Thus we need to hoist the declaration to the parent scope. + hoisted_scope_id = self + .scopes + .get(self.scopes.len() - 2) + .map(|scope| scope.scope_id); + self.push_binding( + hoisted_scope_id, + BindingName::Value(name.clone()), + name_range, + ); + self.push_binding(hoisted_scope_id, BindingName::Type(name), name_range); + } + AnyJsBindingDeclaration::TsInterfaceDeclaration(_) + | AnyJsBindingDeclaration::TsTypeAliasDeclaration(_) => { + // These declarations have their own scope. + // Thus we need to hoist the declaration to the parent scope. + hoisted_scope_id = self + .scopes + .get(self.scopes.len() - 2) + .map(|scope| scope.scope_id); + self.push_binding(hoisted_scope_id, BindingName::Type(name), name_range); + } + AnyJsBindingDeclaration::TsModuleDeclaration(_) => { + // This declarations has its own scope. + // Thus we need to hoist the declaration to the parent scope. + hoisted_scope_id = self + .scopes + .get(self.scopes.len() - 2) + .map(|scope| scope.scope_id); + self.push_binding( + hoisted_scope_id, + BindingName::Value(name.clone()), + name_range, + ); + } + AnyJsBindingDeclaration::TsMappedType(_) + | AnyJsBindingDeclaration::TsTypeParameter(_) => { + self.push_binding(None, BindingName::Type(name), name_range); + } + AnyJsBindingDeclaration::JsImportDefaultClause(clause) => { + if clause.type_token().is_some() { + self.push_binding(None, BindingName::Type(name), name_range); + } else { + self.push_binding(None, BindingName::Value(name.clone()), name_range); + self.push_binding(None, BindingName::Type(name), name_range); + } + } + AnyJsBindingDeclaration::JsImportNamespaceClause(clause) => { + if clause.type_token().is_some() { + self.push_binding(None, BindingName::Type(name), name_range); + } else { + self.push_binding(None, BindingName::Value(name.clone()), name_range); + self.push_binding(None, BindingName::Type(name), name_range); + } + } + AnyJsBindingDeclaration::TsImportEqualsDeclaration(declaration) => { + if declaration.type_token().is_some() { + self.push_binding(None, BindingName::Type(name), name_range); + } else { + self.push_binding(None, BindingName::Value(name.clone()), name_range); + self.push_binding(None, BindingName::Type(name), name_range); + } + } + AnyJsBindingDeclaration::JsDefaultImportSpecifier(_) + | AnyJsBindingDeclaration::JsNamespaceImportSpecifier(_) => { + let clause = declaration.parent::()?; + if clause.type_token().is_some() { + self.push_binding(None, BindingName::Type(name), name_range); + } else { + self.push_binding(None, BindingName::Value(name.clone()), name_range); + self.push_binding(None, BindingName::Type(name), name_range); + } + } + AnyJsBindingDeclaration::JsBogusNamedImportSpecifier(_) + | AnyJsBindingDeclaration::JsShorthandNamedImportSpecifier(_) + | AnyJsBindingDeclaration::JsNamedImportSpecifier(_) => { + let specifier = + AnyJsNamedImportSpecifier::unwrap_cast(declaration.into_syntax()); + if specifier.imports_only_types() { + self.push_binding(None, BindingName::Type(name), name_range); + } else { + self.push_binding(None, BindingName::Value(name.clone()), name_range); + self.push_binding(None, BindingName::Type(name), name_range); + } + } + AnyJsBindingDeclaration::JsArrowFunctionExpression(_) + | AnyJsBindingDeclaration::JsBogusParameter(_) + | AnyJsBindingDeclaration::JsFormalParameter(_) + | AnyJsBindingDeclaration::JsRestParameter(_) + | AnyJsBindingDeclaration::TsIndexSignatureParameter(_) + | AnyJsBindingDeclaration::TsPropertyParameter(_) + | AnyJsBindingDeclaration::JsCatchDeclaration(_) => { + self.push_binding(None, BindingName::Value(name), name_range); + } + AnyJsBindingDeclaration::TsInferType(_) => { + // Delay the declaration of parameter types that are inferred. + // Their scope corresponds to the true branch of the conditional type. + self.infers + .push(TsTypeParameterName::unwrap_cast(node.syntax().clone())); + return Some(()); } } - _ => { - self.push_binding_into_scope(None, &name_token, parent_kind); - } + is_exported + } else { + // Handle identifiers in bogus nodes, + self.push_binding(None, BindingName::Value(name), name_range); + false + }; + let scope_id = self.current_scope_mut().scope_id; + self.stash.push_back(SemanticEvent::DeclarationFound { + scope_id, + hoisted_scope_id, + name_token, + }); + if is_exported { + self.stash.push_back(SemanticEvent::Exported { + range: node.syntax().text_range(), + }); } - Some(()) } - fn enter_reference_identifier(&mut self, node: &JsSyntaxNode) -> Option<()> { - debug_assert!( - matches!( - node.kind(), - JsSyntaxKind::JS_REFERENCE_IDENTIFIER | JsSyntaxKind::JSX_REFERENCE_IDENTIFIER - ), - "specified node is not a reference identifier (JS_REFERENCE_IDENTIFIER, JSX_REFERENCE_IDENTIFIER)" - ); - - let (name, is_exported) = match node.kind() { - JsSyntaxKind::JS_REFERENCE_IDENTIFIER => { - // SAFETY: kind check above - let reference = JsReferenceIdentifier::unwrap_cast(node.clone()); - let name_token = reference.value_token().ok()?; - // skip `this` reference representing the class instance - if name_token.text_trimmed() == "this" { - return None; + fn enter_identifier_usage(&mut self, node: AnyJsIdentifierUsage) { + let range = node.syntax().text_range(); + let Ok(name_token) = node.value_token() else { + return; + }; + let name = name_token.token_text_trimmed(); + match node { + AnyJsIdentifierUsage::JsReferenceIdentifier(node) => { + if let Some(specifier) = node.parent::() { + if specifier.exports_only_types() { + self.push_reference(BindingName::Type(name), Reference::ExportType(range)); + } else { + self.push_reference( + BindingName::Value(name.clone()), + Reference::Export(range), + ); + self.push_reference(BindingName::Type(name), Reference::Export(range)); + } + } else if matches!( + node.syntax().grand_parent().kind(), + Some(JS_EXPORT_DEFAULT_EXPRESSION_CLAUSE | TS_EXPORT_ASSIGNMENT_CLAUSE) + ) { + self.push_reference(BindingName::Value(name.clone()), Reference::Export(range)); + self.push_reference(BindingName::Type(name), Reference::Export(range)); + } else { + if name.text() == "this" { + // Ignore `this` in typeof position. e.g. `typeof this.prop`. + return; + } + let binding_name = match node + .syntax() + .ancestors() + .skip(1) + .find(|x| x.kind() != TS_QUALIFIED_NAME) + .kind() + { + Some(TS_REFERENCE_TYPE | TS_NAME_WITH_TYPE_ARGUMENTS) => { + BindingName::Type(name) + } + // ignore binding `` from `import().` + Some(TS_IMPORT_TYPE_QUALIFIER) => return, + _ => BindingName::Value(name), + }; + self.push_reference(binding_name, Reference::Read(range)); } - ( - name_token.token_text_trimmed(), - self.is_js_reference_identifier_exported(node), - ) } - JsSyntaxKind::JSX_REFERENCE_IDENTIFIER => { - // SAFETY: kind check above - let reference = JsxReferenceIdentifier::unwrap_cast(node.clone()); - let name_token = reference.value_token().ok()?; - (name_token.token_text_trimmed(), false) + AnyJsIdentifierUsage::JsxReferenceIdentifier(_) => { + self.push_reference(BindingName::Value(name), Reference::Read(range)); } - _ => return None, - }; - - let current_scope = self.current_scope_mut(); - let references = current_scope - .references - .entry(BindingName { name }) - .or_default(); - references.push(Reference::Read { - range: node.text_range(), - is_exported, - }); - - Some(()) - } - - fn enter_js_identifier_assignment(&mut self, reference: &JsIdentifierAssignment) -> Option<()> { - let name = reference.name_token().ok()?; - let name = name.token_text_trimmed(); - let current_scope = self.current_scope_mut(); - let references = current_scope - .references - .entry(BindingName { name }) - .or_default(); - references.push(Reference::Write { - range: reference.syntax().text_range(), - }); - - Some(()) + AnyJsIdentifierUsage::JsIdentifierAssignment(_) => { + self.push_reference(BindingName::Value(name), Reference::Write(range)); + } + } } /// See [SemanticEvent] for a more detailed description /// of which ```SyntaxNode``` generates which events. #[inline] pub fn leave(&mut self, node: &JsSyntaxNode) { - use biome_js_syntax::JsSyntaxKind::*; - match node.kind() { JS_MODULE | JS_SCRIPT => self.pop_scope(node.text_range()), JS_FUNCTION_DECLARATION @@ -518,6 +574,7 @@ impl SemanticEventExtractor { | JS_CATCH_CLAUSE | JS_STATIC_INITIALIZATION_BLOCK_CLASS_MEMBER | TS_DECLARE_FUNCTION_DECLARATION + | TS_DECLARE_FUNCTION_EXPORT_DEFAULT_DECLARATION | TS_INTERFACE_DECLARATION | TS_ENUM_DECLARATION | TS_TYPE_ALIAS_DECLARATION @@ -556,33 +613,33 @@ impl SemanticEventExtractor { 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); + if let Ok(name_token) = infer.ident_token() { + let name = name_token.token_text_trimmed(); + let name_range = name_token.text_range(); + self.push_binding(None, BindingName::Type(name), name_range); + let scope_id = self.current_scope_mut().scope_id; + self.stash.push_back(SemanticEvent::DeclarationFound { + scope_id, + hoisted_scope_id: None, + name_token, + }); } } } fn push_scope(&mut self, range: TextRange, hoisting: ScopeHoisting, is_closure: bool) { - let scope_id = self.next_scope_id; - self.next_scope_id += 1; - - let parent_scope_id = self.scopes.iter().last().map(|x| x.scope_id); - + let scope_id = self.scope_count; + self.scope_count += 1; self.stash.push_back(SemanticEvent::ScopeStarted { range, scope_id, - parent_scope_id, + parent_scope_id: self.scopes.iter().last().map(|x| x.scope_id), is_closure, }); - self.scopes.push(Scope { scope_id, - started_at: range.start(), bindings: vec![], - references: HashMap::new(), + references: FxHashMap::default(), shadowed: vec![], hoisting, }); @@ -595,122 +652,114 @@ impl SemanticEventExtractor { /// 4 - All shadowed declarations are restored. fn pop_scope(&mut self, range: TextRange) { debug_assert!(!self.scopes.is_empty()); + let scope = self.scopes.pop().unwrap(); + let scope_id = scope.scope_id; - if let Some(scope) = self.scopes.pop() { - // Match references and declarations - for (name, mut references) in scope.references { + // Match references and declarations + for (name, mut references) in scope.references { + if let Some(&declared_at) = self.bindings.get(&name) { // If we know the declaration of these reference push the correct events... - if let Some(declared_binding) = self.bindings.get(&name) { - let declaration_at = declared_binding.text_range; - let declaration_syntax_kind = declared_binding.declaration_kind; - - for reference in &references { - let declaration_before_reference = - declaration_at.start() < reference.range().start(); - let e = match (declaration_before_reference, &reference) { - (true, Reference::Read { range, .. }) => SemanticEvent::Read { - range: *range, - declared_at: declaration_at, - scope_id: scope.scope_id, - }, - (false, Reference::Read { range, .. }) => SemanticEvent::HoistedRead { - range: *range, - declared_at: declaration_at, - scope_id: scope.scope_id, - }, - (true, Reference::Write { range }) => SemanticEvent::Write { - range: *range, - declared_at: declaration_at, - scope_id: scope.scope_id, - }, - (false, Reference::Write { range }) => SemanticEvent::HoistedWrite { - range: *range, - declared_at: declaration_at, - scope_id: scope.scope_id, - }, - }; - self.stash.push_back(e); - - match reference { - Reference::Read { is_exported, .. } if *is_exported => { - // Check shadowed bindings to find an exportable binding - let find_exportable_binding = scope.shadowed.iter().find_map( - |(shadowed_ident_name, shadowed_binding_info)| { - if shadowed_ident_name != &name { - return None; - } - - // The order of interface and other bindings is valid in either order - match ( - declaration_syntax_kind, - shadowed_binding_info.declaration_kind, - ) { - ( - JsSyntaxKind::JS_VARIABLE_DECLARATOR - | JsSyntaxKind::JS_CLASS_DECLARATION, - JsSyntaxKind::TS_INTERFACE_DECLARATION, - ) - | ( - JsSyntaxKind::TS_INTERFACE_DECLARATION, - JsSyntaxKind::JS_VARIABLE_DECLARATOR - | JsSyntaxKind::JS_CLASS_DECLARATION, - ) => Some(shadowed_binding_info), - _ => None, - } - }, - ); - if let Some(binding_info) = find_exportable_binding { - self.stash.push_back(SemanticEvent::Exported { - range: binding_info.text_range, - }); + for reference in references { + let declaration_before_reference = + declared_at.start() < reference.range().start(); + let event = match reference { + Reference::Export(range) | Reference::ExportType(range) => { + self.stash + .push_back(SemanticEvent::Exported { range: declared_at }); + if declaration_before_reference { + SemanticEvent::Read { + range, + declared_at, + scope_id, + } + } else { + SemanticEvent::HoistedRead { + range, + declared_at, + scope_id, } - - self.stash.push_back(SemanticEvent::Exported { - range: declaration_at, - }); } - _ => {} } + Reference::Read(range) => { + if declaration_before_reference { + SemanticEvent::Read { + range, + declared_at, + scope_id, + } + } else { + SemanticEvent::HoistedRead { + range, + declared_at, + scope_id, + } + } + } + Reference::Write(range) => { + if declaration_before_reference { + SemanticEvent::Write { + range, + declared_at, + scope_id, + } + } else { + SemanticEvent::HoistedWrite { + range, + declared_at, + scope_id, + } + } + } + }; + self.stash.push_back(event); + } + } else if let Some(parent) = self.scopes.last_mut() { + // ... if not, promote these references to the parent scope ... + let parent_references = parent.references.entry(name).or_default(); + parent_references.append(&mut references); + } else { + // ... or raise UnresolvedReference if this is the global scope. + let has_dual_binding = self.has_dual_binding(name); + for reference in references { + if has_dual_binding && reference.is_export() { + // An export can export both a value and a type. + // If a dual binding exists, then it exports the dual binding. + continue; } - } else if let Some(parent) = self.scopes.last_mut() { - // ... if not, promote these references to the parent scope ... - let parent_references = parent.references.entry(name).or_default(); - parent_references.append(&mut references); - } else { - // ... or raise UnresolvedReference if this is the global scope. - for reference in references { - self.stash.push_back(SemanticEvent::UnresolvedReference { - is_read: reference.is_read(), - range: *reference.range(), - }); - } + self.stash.push_back(SemanticEvent::UnresolvedReference { + is_read: !reference.is_write(), + range: *reference.range(), + }); } } + } - // Remove all bindings declared in this scope - for binding in scope.bindings { - self.bindings.remove(&binding); - } + // Remove all bindings declared in this scope + for binding in scope.bindings { + self.bindings.remove(&binding); + } - // Restore shadowed bindings - self.bindings.extend(scope.shadowed); + // Restore shadowed bindings + self.bindings.extend(scope.shadowed); - self.stash.push_back(SemanticEvent::ScopeEnded { - range, - started_at: scope.started_at, - scope_id: scope.scope_id, - }); - } + self.stash.push_back(SemanticEvent::ScopeEnded { + range, + scope_id: scope.scope_id, + }); + } + + fn has_dual_binding(&self, binding_name: BindingName) -> bool { + let dual_binding_name = match binding_name { + BindingName::Type(name) => BindingName::Value(name), + BindingName::Value(name) => BindingName::Type(name), + }; + self.bindings.contains_key(&dual_binding_name) } fn current_scope_mut(&mut self) -> &mut Scope { // We should at least have the global scope debug_assert!(!self.scopes.is_empty()); - - match self.scopes.last_mut() { - Some(scope) => scope, - None => unreachable!(), - } + self.scopes.last_mut().unwrap() } /// Finds the scope where declarations that are hoisted @@ -730,251 +779,55 @@ impl SemanticEventExtractor { /// This method when called inside the `f` scope will return /// the `f` scope index. fn scope_index_to_hoist_declarations(&mut self, skip: usize) -> Option { + debug_assert!(self.scopes.len() > skip); // We should at least have the global scope // that do not hoist debug_assert!(matches!( self.scopes[0].hoisting, ScopeHoisting::DontHoistDeclarationsToParent )); - debug_assert!(self.scopes.len() > skip); - - let scope_id_hoisted_to = self - .scopes + self.scopes .iter() .rev() .skip(skip) - .find(|scope| matches!(scope.hoisting, ScopeHoisting::DontHoistDeclarationsToParent)) - .map(|x| x.scope_id); - - let current_scope_id = self.current_scope_mut().scope_id; - match scope_id_hoisted_to { - Some(scope_id) => { - if scope_id == current_scope_id { - None - } else { - Some(scope_id) - } - } - // Worst case this will fallback to the global scope - // which will be idx = 0 - None => unreachable!("We must have a least of scope."), - } + .find(|scope| scope.hoisting == ScopeHoisting::DontHoistDeclarationsToParent) + .map(|x| x.scope_id) + .filter(|scope_id| self.current_scope_mut().scope_id != *scope_id) } - fn push_binding_into_scope( + /// Push the binding `binding` into the hoisted scope if it exists, or into the current scope. + fn push_binding( &mut self, hoisted_scope_id: Option, - name_token: &JsSyntaxToken, - declaration_kind: JsSyntaxKind, + binding_name: BindingName, + name_range: TextRange, ) { - let name = name_token.token_text_trimmed(); - let declaration_range = name_token.text_range(); - let binding = BindingName { name: name.clone() }; - // insert this name into the list of available names - // and save shadowed names to be used later - let shadowed = self - .bindings - .insert( - binding.clone(), - BindingInfo { - text_range: declaration_range, - declaration_kind, - }, - ) - .map(|shadowed_range| (binding, shadowed_range)); - let current_scope_id = self.current_scope_mut().scope_id; let binding_scope_id = hoisted_scope_id.unwrap_or(current_scope_id); - let scope = self .scopes .iter_mut() .rev() .find(|s| s.scope_id == binding_scope_id); - // A scope will always be found debug_assert!(scope.is_some()); let scope = scope.unwrap(); - scope.bindings.push(BindingName { name: name.clone() }); - scope.shadowed.extend(shadowed); - - self.stash.push_back(SemanticEvent::DeclarationFound { - range: declaration_range, - scope_started_at: scope.started_at, - scope_id: current_scope_id, - hoisted_scope_id, - name, - }); - } - - // Check if a function is exported and raise the [Exported] event. - fn export_function_declaration( - &mut self, - binding: &JsSyntaxNode, - function_declaration: &JsSyntaxNode, - ) { - use JsSyntaxKind::*; - debug_assert!(matches!( - function_declaration.kind(), - JS_FUNCTION_DECLARATION | JS_FUNCTION_EXPORT_DEFAULT_DECLARATION - )); - let is_exported = matches!( - function_declaration.parent().kind(), - Some(JS_EXPORT | JS_EXPORT_DEFAULT_DECLARATION_CLAUSE) - ); - if is_exported { - self.stash.push_back(SemanticEvent::Exported { - range: binding.text_range(), - }); - } - } - - // Check if a function or class expression is exported and raise the [Exported] event. - fn export_declaration_expression( - &mut self, - binding: &JsSyntaxNode, - declaration_expression: &JsSyntaxNode, - ) { - use JsSyntaxKind::*; - debug_assert!(matches!( - declaration_expression.kind(), - JS_FUNCTION_EXPRESSION | JS_CLASS_EXPRESSION - )); - let is_module_exports = declaration_expression - .parent() - .map(|x| self.is_assignment_left_side_module_exports(&x)) - .unwrap_or(false); - if is_module_exports { - self.stash.push_back(SemanticEvent::Exported { - range: binding.text_range(), - }); - } - } - - // Check if a class, type alias, enum, interface, module is exported and raise the [Exported] event. - fn export_declaration(&mut self, binding: &JsSyntaxNode, declaration: &JsSyntaxNode) { - use JsSyntaxKind::*; - debug_assert!(matches!( - declaration.kind(), - JS_CLASS_DECLARATION - | JS_CLASS_EXPORT_DEFAULT_DECLARATION - | TS_TYPE_ALIAS_DECLARATION - | TS_ENUM_DECLARATION - | TS_INTERFACE_DECLARATION - | TS_MODULE_DECLARATION - | TS_EXTERNAL_MODULE_DECLARATION - )); - let is_exported = matches!( - declaration.parent().kind(), - Some(JS_EXPORT | JS_EXPORT_DEFAULT_DECLARATION_CLAUSE) - ); - - if is_exported { - self.stash.push_back(SemanticEvent::Exported { - range: binding.text_range(), - }); - } - } - - // Check if a variable is exported and raise the [Exported] event. - fn export_variable_declarator( - &mut self, - binding: &JsSyntaxNode, - variable_declarator: &JsSyntaxNode, - ) { - use JsSyntaxKind::*; - debug_assert!(matches!(variable_declarator.kind(), JS_VARIABLE_DECLARATOR)); - let is_exported = matches!( - variable_declarator - .parent() - .and_then(|list| list.parent()) - .and_then(|declaration| declaration.parent()) - .and_then(|declaration_clause| declaration_clause.parent()) - .map(|x| x.kind()), - Some(JS_EXPORT | TS_EXPORT_DECLARE_CLAUSE) - ); - - if is_exported { - self.stash.push_back(SemanticEvent::Exported { - range: binding.text_range(), - }); - } - } - - // Check if a reference is exported and raise the [Exported] event. - fn is_js_reference_identifier_exported(&mut self, reference: &JsSyntaxNode) -> bool { - use JsSyntaxKind::*; - debug_assert!(matches!(reference.kind(), JS_REFERENCE_IDENTIFIER)); - let reference_parent = reference.parent(); - let reference_greatparent = reference_parent.as_ref().and_then(|p| p.parent()); - - // check export keyword - matches!( - reference_parent.kind(), - Some(JS_EXPORT_NAMED_SHORTHAND_SPECIFIER | JS_EXPORT_NAMED_SPECIFIER) - ) | { - // check "export default" keyword - matches!( - reference_greatparent.kind(), - Some(JS_EXPORT_DEFAULT_EXPRESSION_CLAUSE) - ) - } | { - // check module.exports - match reference_parent.map(|x| x.kind()) { - Some(JS_IDENTIFIER_EXPRESSION) => reference_greatparent - .map(|x| self.is_assignment_left_side_module_exports(&x)) - .unwrap_or(false), - Some(JS_SHORTHAND_PROPERTY_OBJECT_MEMBER) => reference_greatparent - .and_then(|g| g.grand_parent()) - .map(|x| self.is_assignment_left_side_module_exports(&x)) - .unwrap_or(false), - _ => false, - } + // insert this name into the list of available names + // and save shadowed names to be used later + if let Some(shadowed) = self.bindings.insert(binding_name.clone(), name_range) { + scope.shadowed.push((binding_name.clone(), shadowed)); } + scope.bindings.push(binding_name); } - fn is_assignment_left_side_module_exports(&self, node: &JsSyntaxNode) -> bool { - match node.kind() { - JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION => { - let expr = JsAssignmentExpression::unwrap_cast(node.clone()); - match expr.left() { - Ok(AnyJsAssignmentPattern::AnyJsAssignment( - AnyJsAssignment::JsStaticMemberAssignment(a), - )) => { - let first = a - .object() - .ok() - .and_then(|x| x.as_js_identifier_expression().cloned()) - .and_then(|x| x.name().ok()) - .and_then(|x| x.value_token().ok()) - .map(|x| x.token_text_trimmed()); - - match first { - Some(first) if first == "module" => { - let second = a - .member() - .ok() - .and_then(|x| x.as_js_name().cloned()) - .and_then(|x| x.value_token().ok()) - .map(|x| x.token_text_trimmed()); - // module.exports = .. - matches!(second, Some(second) if second == "exports") - } - // exports. = .. - Some(first) if first == "exports" => true, - _ => false, - } - } - // exports = ... - Ok(AnyJsAssignmentPattern::AnyJsAssignment( - AnyJsAssignment::JsIdentifierAssignment(ident), - )) => ident.syntax().text_trimmed() == "exports", - _ => false, - } - } - _ => false, - } + /// Push the reference `reference` of the binding `binding_name` into the current scope. + fn push_reference(&mut self, binding_name: BindingName, reference: Reference) { + self.current_scope_mut() + .references + .entry(binding_name) + .or_default() + .push(reference); } } diff --git a/crates/biome_js_semantic/src/semantic_model/builder.rs b/crates/biome_js_semantic/src/semantic_model/builder.rs index 93a311120605..9fe513c6b2ae 100644 --- a/crates/biome_js_semantic/src/semantic_model/builder.rs +++ b/crates/biome_js_semantic/src/semantic_model/builder.rs @@ -137,12 +137,12 @@ impl SemanticModelBuilder { } ScopeEnded { .. } => {} DeclarationFound { - name, - range, + name_token, scope_id, hoisted_scope_id, - .. } => { + let name_range = name_token.text_range(); + let name = name_token.token_text_trimmed(); let binding_scope_id = hoisted_scope_id.unwrap_or(scope_id); // SAFETY: this scope id is guaranteed to exist because they were generated by the @@ -152,10 +152,10 @@ impl SemanticModelBuilder { let binding_id = self.bindings.len(); self.bindings.push(SemanticModelBindingData { id: binding_id.into(), - range, + range: name_range, references: vec![], }); - self.bindings_by_range.insert(range, binding_id); + self.bindings_by_range.insert(name_range, binding_id); let scope = self.scopes.get_mut(binding_scope_id).unwrap(); @@ -164,7 +164,7 @@ impl SemanticModelBuilder { if let Some(hoisted_scope_id) = hoisted_scope_id { self.scope_hoisted_to_by_range - .insert(range.start(), hoisted_scope_id); + .insert(name_range.start(), hoisted_scope_id); } } Read { diff --git a/crates/biome_js_semantic/src/semantic_model/tests.rs b/crates/biome_js_semantic/src/semantic_model/tests.rs index a509c161434b..a2571988477c 100644 --- a/crates/biome_js_semantic/src/semantic_model/tests.rs +++ b/crates/biome_js_semantic/src/semantic_model/tests.rs @@ -199,18 +199,7 @@ mod test { ); } JsSyntaxKind::JS_REFERENCE_IDENTIFIER => { - let reference = JsReferenceIdentifier::cast(node).unwrap(); - // These do the same thing, but with different APIs - assert!( - is_exported == model.is_exported(&reference).unwrap(), - "at \"{}\"", - code - ); - assert!( - is_exported == reference.is_exported(&model).unwrap(), - "at \"{}\"", - code - ); + // Do nothings. } x => { panic!("This node cannot be exported! {:?}", x); @@ -225,10 +214,8 @@ mod test { assert_is_exported(true, "A", "export const A = 1"); assert_is_exported(true, "A", "const A = 1; export default A"); assert_is_exported(true, "A", "const A = 1; export {A}"); - assert_is_exported(true, "A", "const A = 1; module.exports = A;"); - assert_is_exported(true, "A", "const A = 1; module.exports = {A};"); - assert_is_exported(true, "A", "const A = 1; exports = A;"); - assert_is_exported(true, "A", "const A = 1; exports.A = A;"); + assert_is_exported(false, "A", "const A = 1; export {type A}"); + assert_is_exported(false, "A", "const A = 1; export type {A}"); // Functions assert_is_exported(false, "f", "function f() {}"); @@ -236,27 +223,19 @@ mod test { assert_is_exported(true, "f", "export default function f() {}"); assert_is_exported(true, "f", "function f() {} export default f"); assert_is_exported(true, "f", "function f() {} export {f}"); + assert_is_exported(false, "f", "function f() {} export {type f}"); + assert_is_exported(false, "f", "function f() {} export type {f}"); assert_is_exported(true, "f", "function f() {} export {f as g}"); - assert_is_exported(true, "f", "module.exports = function f() {}"); - assert_is_exported(true, "f", "exports = function f() {}"); - assert_is_exported(true, "f", "exports.f = function f() {}"); - assert_is_exported(true, "f", "function f() {} module.exports = f"); - assert_is_exported(true, "f", "function f() {} module.exports = {f}"); - assert_is_exported(true, "f", "function f() {} exports = f"); - assert_is_exported(true, "f", "function f() {} exports.f = f"); - - // Classess + + // Classes assert_is_exported(false, "A", "class A{}"); assert_is_exported(true, "A", "export class A{}"); assert_is_exported(true, "A", "export default class A{}"); assert_is_exported(true, "A", "class A{} export default A"); assert_is_exported(true, "A", "class A{} export {A}"); + assert_is_exported(true, "A", "class A{} export {type A}"); assert_is_exported(true, "A", "class A{} export {A as B}"); - assert_is_exported(true, "A", "module.exports = class A{}"); - assert_is_exported(true, "A", "exports = class A{}"); - assert_is_exported(true, "A", "class A{} module.exports = A"); - assert_is_exported(true, "A", "class A{} exports = A"); - assert_is_exported(true, "A", "class A{} exports.A = A"); + assert_is_exported(true, "A", "class A{} export {type A as B}"); // Interfaces assert_is_exported(false, "A", "interface A{}"); @@ -264,30 +243,30 @@ mod test { assert_is_exported(true, "A", "export default interface A{}"); assert_is_exported(true, "A", "interface A{} export default A"); assert_is_exported(true, "A", "interface A{} export {A}"); + assert_is_exported(true, "A", "interface A{} export {type A}"); + assert_is_exported(true, "A", "interface A{} export type {A}"); assert_is_exported(true, "A", "interface A{} export {A as B}"); - assert_is_exported(true, "A", "interface A{} module.exports = A"); - assert_is_exported(true, "A", "interface A{} exports = A"); - assert_is_exported(true, "A", "interface A{} exports.A = A"); + assert_is_exported(true, "A", "interface A{} export {type A as B}"); // Type Aliases assert_is_exported(false, "A", "type A = number;"); assert_is_exported(true, "A", "export type A = number;"); assert_is_exported(true, "A", "type A = number; export default A"); assert_is_exported(true, "A", "type A = number; export {A}"); + assert_is_exported(true, "A", "type A = number; export {type A}"); + assert_is_exported(true, "A", "type A = number; export type {A}"); assert_is_exported(true, "A", "type A = number; export {A as B}"); - assert_is_exported(true, "A", "type A = number; module.exports = A"); - assert_is_exported(true, "A", "type A = number; exports = A"); - assert_is_exported(true, "A", "type A = number; exports.A = A"); + assert_is_exported(true, "A", "type A = number; export {type A as B}"); // Enums assert_is_exported(false, "A", "enum A {};"); assert_is_exported(true, "A", "export enum A {};"); assert_is_exported(true, "A", "enum A {}; export default A"); assert_is_exported(true, "A", "enum A {}; export {A}"); + assert_is_exported(true, "A", "enum A {}; export {type A}"); + assert_is_exported(true, "A", "enum A {}; export type {A}"); assert_is_exported(true, "A", "enum A {}; export {A as B}"); - assert_is_exported(true, "A", "enum A {}; module.exports = A"); - assert_is_exported(true, "A", "enum A {}; exports = A"); - assert_is_exported(true, "A", "enum A {}; exports.A = A"); + assert_is_exported(true, "A", "enum A {}; export {type A as B}"); } #[test] diff --git a/crates/biome_js_semantic/src/tests/assertions.rs b/crates/biome_js_semantic/src/tests/assertions.rs index a4b68a12c0d7..a5fa485d61e4 100644 --- a/crates/biome_js_semantic/src/tests/assertions.rs +++ b/crates/biome_js_semantic/src/tests/assertions.rs @@ -7,6 +7,7 @@ use biome_diagnostics::{ use biome_js_parser::JsParserOptions; use biome_js_syntax::{AnyJsRoot, JsFileSource, JsSyntaxToken, TextRange, TextSize, WalkEvent}; use biome_rowan::{AstNode, NodeOrToken}; +use rustc_hash::FxHashMap; use std::collections::{BTreeMap, HashMap}; /// This method helps testing scope resolution. It does this @@ -123,19 +124,17 @@ pub fn assert(code: &str, test_name: &str) { // Extract semantic events and index by range let mut events_by_pos: HashMap> = HashMap::new(); + let mut scope_start_by_id: FxHashMap = FxHashMap::default(); for event in semantic_events(r.syntax()) { - let pos = match &event { - SemanticEvent::DeclarationFound { range, .. } => range.start(), - SemanticEvent::ScopeStarted { range, .. } => range.start(), - SemanticEvent::ScopeEnded { range, .. } => range.end(), - SemanticEvent::Read { range, .. } => range.start(), - SemanticEvent::HoistedRead { range, .. } => range.start(), - SemanticEvent::Write { range, .. } => range.start(), - SemanticEvent::HoistedWrite { range, .. } => range.start(), - SemanticEvent::UnresolvedReference { range, .. } => range.start(), - SemanticEvent::Exported { range } => range.start(), + let pos = if let SemanticEvent::ScopeEnded { + range, scope_id, .. + } = event + { + scope_start_by_id.insert(scope_id, range.start()); + range.end() + } else { + event.range().start() }; - let v = events_by_pos.entry(pos).or_default(); v.push(event); } @@ -144,7 +143,7 @@ pub fn assert(code: &str, test_name: &str) { // check - assertions.check(code, test_name, events_by_pos); + assertions.check(code, test_name, events_by_pos, scope_start_by_id); } #[derive(Debug, Diagnostic)] @@ -448,6 +447,7 @@ impl SemanticAssertions { code: &str, test_name: &str, events_by_pos: HashMap>, + scope_start: FxHashMap, ) { // Check every declaration assertion is ok @@ -607,12 +607,17 @@ impl SemanticAssertions { // Needs to be a unique event for now match &events[0] { SemanticEvent::DeclarationFound { - scope_started_at, .. + scope_id, + hoisted_scope_id, + .. } => match self .scope_start_assertions .get(&at_scope_assertion.scope_name) { Some(scope_start_assertion) => { + let scope_started_at = scope_start + .get(&hoisted_scope_id.unwrap_or(*scope_id)) + .unwrap(); if scope_start_assertion.range.start() != *scope_started_at { show_all_events(test_name, code, events_by_pos, is_scope_event); show_unmatched_assertion( @@ -686,8 +691,8 @@ impl SemanticAssertions { // At least one of the events should be a scope start starting // where we expect let e = events.iter().find(|event| match event { - SemanticEvent::ScopeEnded { started_at, .. } => { - *started_at == scope_start_assertions_range.start() + SemanticEvent::ScopeEnded { range, .. } => { + range.start() == scope_start_assertions_range.start() } _ => false, }); diff --git a/crates/biome_js_semantic/src/tests/scopes.rs b/crates/biome_js_semantic/src/tests/scopes.rs index a747657ddf99..facb4e733eb7 100644 --- a/crates/biome_js_semantic/src/tests/scopes.rs +++ b/crates/biome_js_semantic/src/tests/scopes.rs @@ -58,7 +58,7 @@ assert_semantics! { ", ok_type_parameter_interface, " export interface /*START A*/ EventHandler { - [`on${ Event /*READ Event */ }`]: (event: Event /*READ Event */, data: unknown) => void; + on: (event: Event /*READ Event */, data: unknown) => void; } /*END A*/ ", } diff --git a/crates/biome_js_syntax/src/binding_ext.rs b/crates/biome_js_syntax/src/binding_ext.rs index 0eda0edebbda..778b889ed150 100644 --- a/crates/biome_js_syntax/src/binding_ext.rs +++ b/crates/biome_js_syntax/src/binding_ext.rs @@ -2,7 +2,7 @@ use crate::{ JsArrowFunctionExpression, JsBogusNamedImportSpecifier, JsBogusParameter, JsCatchDeclaration, JsClassDeclaration, JsClassExportDefaultDeclaration, JsClassExpression, JsConstructorClassMember, JsConstructorParameterList, JsConstructorParameters, - JsDefaultImportSpecifier, JsFormalParameter, JsFunctionDeclaration, + JsDefaultImportSpecifier, JsExport, JsFormalParameter, JsFunctionDeclaration, JsFunctionExportDefaultDeclaration, JsFunctionExpression, JsIdentifierBinding, JsImportDefaultClause, JsImportNamespaceClause, JsMethodClassMember, JsMethodObjectMember, JsNamedImportSpecifier, JsNamespaceImportSpecifier, JsParameterList, JsParameters, @@ -23,7 +23,7 @@ declare_node_union! { // variable JsVariableDeclarator // parameters - | JsFormalParameter | JsRestParameter | JsBogusParameter + | JsArrowFunctionExpression | JsFormalParameter | JsRestParameter | JsBogusParameter | TsIndexSignatureParameter | TsPropertyParameter // type parameter | TsInferType | TsMappedType | TsTypeParameter @@ -152,12 +152,39 @@ impl AnyJsBindingDeclaration { pub const fn is_parameter_like(&self) -> bool { matches!( self, - AnyJsBindingDeclaration::JsFormalParameter(_) + AnyJsBindingDeclaration::JsArrowFunctionExpression(_) + | AnyJsBindingDeclaration::JsFormalParameter(_) | AnyJsBindingDeclaration::JsRestParameter(_) | AnyJsBindingDeclaration::JsBogusParameter(_) | AnyJsBindingDeclaration::TsPropertyParameter(_) ) } + + /// Returns the export statement if this declaration is directly exported. + pub fn export(&self) -> Option { + let maybe_export = match self { + Self::JsVariableDeclarator(_) => self.syntax().ancestors().nth(4), + Self::JsFunctionDeclaration(_) + | Self::JsClassDeclaration(_) + | Self::TsTypeAliasDeclaration(_) + | Self::TsEnumDeclaration(_) + | Self::TsModuleDeclaration(_) => self.syntax().parent(), + Self::TsInterfaceDeclaration(_) => { + // interfaces can be in a default export clause + // `export default interface I {}` + self.syntax() + .ancestors() + .skip(1) + .find(|x| x.kind() != JsSyntaxKind::JS_EXPORT_DEFAULT_DECLARATION_CLAUSE) + } + Self::JsClassExportDefaultDeclaration(_) + | Self::JsFunctionExportDefaultDeclaration(_) + | Self::TsDeclareFunctionDeclaration(_) + | Self::TsDeclareFunctionExportDefaultDeclaration(_) => self.syntax().grand_parent(), + _ => None, + }; + maybe_export.and_then(JsExport::cast) + } } declare_node_union! { @@ -243,12 +270,7 @@ impl AnyJsIdentifierBinding { } pub fn declaration(&self) -> Option { - let node = match self { - AnyJsIdentifierBinding::JsIdentifierBinding(binding) => &binding.syntax, - AnyJsIdentifierBinding::TsIdentifierBinding(binding) => &binding.syntax, - AnyJsIdentifierBinding::TsTypeParameterName(binding) => &binding.syntax, - }; - declaration(node) + declaration(self.syntax()) } pub fn is_under_pattern_binding(&self) -> Option { diff --git a/crates/biome_js_syntax/src/export_ext.rs b/crates/biome_js_syntax/src/export_ext.rs new file mode 100644 index 000000000000..4f97b37c8038 --- /dev/null +++ b/crates/biome_js_syntax/src/export_ext.rs @@ -0,0 +1,32 @@ +use biome_rowan::AstNode; + +use crate::{AnyJsExportNamedSpecifier, JsExportNamedClause, JsSyntaxToken}; + +impl AnyJsExportNamedSpecifier { + /// Type token of the export specifier. + /// + /// ```ts + /// export { type X } + /// ^^^^ + /// ``` + pub fn type_token(&self) -> Option { + match self { + Self::JsExportNamedShorthandSpecifier(specifier) => specifier.type_token(), + Self::JsExportNamedSpecifier(specifier) => specifier.type_token(), + } + } + + /// Returns the export clause that includes this specifier. + pub fn export_named_clause(&self) -> Option { + JsExportNamedClause::cast(self.syntax().grand_parent()?) + } + + /// Returns `true` if this specifier or its export clause has **only** a type modifier. + pub fn exports_only_types(&self) -> bool { + self.type_token().is_some() + || self + .export_named_clause() + .and_then(|x| x.type_token()) + .is_some() + } +} diff --git a/crates/biome_js_syntax/src/import_ext.rs b/crates/biome_js_syntax/src/import_ext.rs index e89715573ef6..3e82fad1bc6d 100644 --- a/crates/biome_js_syntax/src/import_ext.rs +++ b/crates/biome_js_syntax/src/import_ext.rs @@ -1,8 +1,8 @@ use crate::{ - inner_string_text, AnyJsImportClause, AnyJsNamedImportSpecifier, JsImport, JsModuleSource, - JsSyntaxToken, + inner_string_text, AnyJsImportClause, AnyJsNamedImportSpecifier, JsImport, JsImportNamedClause, + JsModuleSource, JsSyntaxToken, }; -use biome_rowan::{SyntaxResult, TokenText}; +use biome_rowan::{AstNode, SyntaxResult, TokenText}; impl JsImport { /// It checks if the source of an import against the string `source_to_check` @@ -77,6 +77,36 @@ impl AnyJsNamedImportSpecifier { } } +impl AnyJsNamedImportSpecifier { + /// Type token of the import specifier. + /// + /// ```ts + /// import { type X } + /// ^^^^ + /// ``` + pub fn type_token(&self) -> Option { + match self { + Self::JsBogusNamedImportSpecifier(_) => None, + Self::JsNamedImportSpecifier(specifier) => specifier.type_token(), + Self::JsShorthandNamedImportSpecifier(specifier) => specifier.type_token(), + } + } + + /// Returns the import clause that includes this specifier. + pub fn import_named_clause(&self) -> Option { + JsImportNamedClause::cast(self.syntax().ancestors().nth(3)?) + } + + /// Returns `true` if this specifier or its import clause has **only** a type modifier. + pub fn imports_only_types(&self) -> bool { + self.type_token().is_some() + || self + .import_named_clause() + .and_then(|x| x.type_token()) + .is_some() + } +} + impl JsModuleSource { /// Get the inner text of a string not including the quotes /// ## Examples diff --git a/crates/biome_js_syntax/src/lib.rs b/crates/biome_js_syntax/src/lib.rs index 17de43a28b39..86ea9ece581f 100644 --- a/crates/biome_js_syntax/src/lib.rs +++ b/crates/biome_js_syntax/src/lib.rs @@ -7,6 +7,7 @@ mod generated; pub mod binding_ext; pub mod declaration_ext; pub mod directive_ext; +pub mod export_ext; pub mod expr_ext; pub mod file_source; pub mod function_ext; @@ -28,6 +29,7 @@ pub use biome_rowan::{ SyntaxNodeText, TextLen, TextRange, TextSize, TokenAtOffset, TokenText, TriviaPieceKind, WalkEvent, }; +pub use export_ext::*; pub use expr_ext::*; pub use file_source::*; pub use function_ext::*; diff --git a/crates/biome_js_syntax/src/stmt_ext.rs b/crates/biome_js_syntax/src/stmt_ext.rs index b6622bf0a224..f2826953d0f3 100644 --- a/crates/biome_js_syntax/src/stmt_ext.rs +++ b/crates/biome_js_syntax/src/stmt_ext.rs @@ -2,10 +2,10 @@ use crate::{ AnyJsArrayAssignmentPatternElement, AnyJsAssignmentPattern, AnyJsSwitchClause, - JsForVariableDeclaration, JsStatementList, JsSyntaxToken as SyntaxToken, JsVariableDeclaration, - TsModuleDeclaration, T, + JsForVariableDeclaration, JsStatementList, JsSyntaxKind, JsSyntaxToken as SyntaxToken, + JsVariableDeclaration, JsVariableDeclarator, TsModuleDeclaration, T, }; -use biome_rowan::{declare_node_union, SyntaxResult}; +use biome_rowan::{declare_node_union, AstNode, SyntaxResult}; impl AnyJsSwitchClause { pub fn clause_token(&self) -> SyntaxResult { @@ -129,6 +129,17 @@ impl AnyJsVariableDeclaration { } } +impl JsVariableDeclarator { + /// Variable declaration associated to this declarator. + pub fn declaration(&self) -> Option { + self.syntax() + .ancestors() + .skip(1) + .find(|x| x.kind() != JsSyntaxKind::JS_VARIABLE_DECLARATOR_LIST) + .and_then(AnyJsVariableDeclaration::cast) + } +} + impl AnyJsArrayAssignmentPatternElement { pub fn pattern(self) -> Option { match self { diff --git a/crates/biome_js_syntax/src/type_ext.rs b/crates/biome_js_syntax/src/type_ext.rs index 6386d226d4b3..c31fd8a8f21e 100644 --- a/crates/biome_js_syntax/src/type_ext.rs +++ b/crates/biome_js_syntax/src/type_ext.rs @@ -1,7 +1,7 @@ use biome_rowan::AstNode; use std::iter; -use crate::{AnyTsType, TsConditionalType, TsInferType, TsTypeParameterName}; +use crate::{AnyTsType, TsConditionalType}; impl AnyTsType { /// Try to extract non `TsParenthesizedType` from `AnyTsType` @@ -114,22 +114,3 @@ impl AnyTsType { .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::() - .and_then(|parent| parent.name().ok()) - .map_or(false, |ref name| name == self) - } -} diff --git a/crates/biome_rowan/src/macros.rs b/crates/biome_rowan/src/macros.rs index 1bf3bce326a0..db15c79fe2e4 100644 --- a/crates/biome_rowan/src/macros.rs +++ b/crates/biome_rowan/src/macros.rs @@ -140,7 +140,7 @@ macro_rules! declare_node_union { }; } -/// This trait is implemented for tuples of AstNode types of size 1 to 12 if +/// This trait is implemented for tuples of AstNode types of size 1 to 32 if /// all node types share the same associated language (which is then aliased as /// the `Language` associated type on [UnionLanguage] itself) pub trait UnionLanguage { @@ -164,5 +164,5 @@ macro_rules! impl_union_language { impl_union_language!( T00, T01, T02, T03, T04, T05, T06, T07, T08, T09, T10, T11, T12, T13, T14, T15, T16, T17, T18, - T19, T20, T21, T22, T23, T24, T25, T26, T27, T28, T29 + T19, T20, T21, T22, T23, T24, T25, T26, T27, T28, T29, T30, T31 ); diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index 4c22adce6967..73bfcf461dfd 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -159,6 +159,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [452](https://github.com/biomejs/biome/pull/452). The linter panicked when it met a malformed regex (a regex not ending with a slash). +- Fix [#104](https://github.com/biomejs/biome/issues/104). We now correctly handle types and values with the same name. + ### Parser - Enhance diagnostic for infer type handling in the parser. The 'infer' keyword can only be utilized within the 'extends' clause of a conditional type. Using it outside of this context will result in an error. Ensure that any type declarations using 'infer' are correctly placed within the conditional type structure to avoid parsing issues. Contributed by @denbezrukov diff --git a/xtask/coverage/src/symbols/msts.rs b/xtask/coverage/src/symbols/msts.rs index 745826a148f1..47ff0622f9bb 100644 --- a/xtask/coverage/src/symbols/msts.rs +++ b/xtask/coverage/src/symbols/msts.rs @@ -1,10 +1,12 @@ use biome_js_semantic::SemanticEvent; use biome_js_syntax::JsFileSource; +use biome_rowan::TextSize; use super::utils::{parse_separated_list, parse_str, parse_until_chr, parse_whitespace0}; use crate::check_file_encoding; use crate::runner::{TestCase, TestCaseFiles, TestRunOutcome, TestSuite}; use biome_js_parser::JsParserOptions; +use std::collections::HashSet; use std::fmt::Write; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -79,6 +81,7 @@ impl TestCase for SymbolsMicrosoftTestCase { options.clone(), ); + let mut prev_starts: HashSet = HashSet::default(); let r = biome_js_parser::parse(&code, JsFileSource::tsx(), options); let mut actual: Vec<_> = biome_js_semantic::semantic_events(r.syntax()) .into_iter() @@ -91,13 +94,20 @@ impl TestCase for SymbolsMicrosoftTestCase { | SemanticEvent::Read { .. } | SemanticEvent::HoistedRead { .. } | SemanticEvent::Write { .. } - | SemanticEvent::HoistedWrite { .. } => { - let name = x.str(&code); + | SemanticEvent::HoistedWrite { .. } + | SemanticEvent::UnresolvedReference { .. } => { + let name = &code[x.range()]; !name.contains('\"') && !name.contains('\'') } - _ => false, + SemanticEvent::ScopeStarted { .. } + | SemanticEvent::ScopeEnded { .. } + | SemanticEvent::Exported { .. } => false, } }) + .filter(|x| { + // Ignore the current event if one was already processed for the same range. + prev_starts.insert(x.range().start()) + }) .collect(); actual.sort_by_key(|x| x.range().start()); @@ -125,12 +135,12 @@ impl TestCase for SymbolsMicrosoftTestCase { debug_text.push_str(" - actual: "); if let Some(actual) = actual { - let name = actual.str(&code).trim(); + let name = &code[actual.range()].trim(); write!(debug_text, "[{}]", name).unwrap(); } match (expected, actual) { - (Some(expected), Some(actual)) if expected.name != actual.str(&code).trim() => { + (Some(expected), Some(actual)) if expected.name != code[actual.range()].trim() => { debug_text.push_str(" <<<<<<<<<<<<<<<<<<<< Diff here"); } _ => {} @@ -146,7 +156,7 @@ impl TestCase for SymbolsMicrosoftTestCase { } } else { for (expected, actual) in expected.symbols.iter().zip(actual) { - let are_names_eq = expected.name == actual.str(&code).trim(); + let are_names_eq = expected.name == code[actual.range()].trim(); if !are_names_eq { return TestRunOutcome::IncorrectlyErrored { files: t, @@ -222,12 +232,7 @@ struct SymbolsFile { fn parse_symbol(input: &str) -> Option { let (input, _) = parse_str(input, ">")?; let (input, name) = parse_until_chr(input, |x| x.is_whitespace() || x == ':')?; - if name.contains('.') - || name.contains('[') - || name.contains('\"') - || name.contains('\'') - || name == "undefined" - { + if name.contains('.') || name.contains('[') || name.contains('\"') || name.contains('\'') { return None; } let (input, _) = parse_whitespace0(input);