Set up Export SymbolFlag and store declaration AST in Symbol

crates/oxc_linter/src/rules/typescript/isolated_declaration.rs

@@ -1,19 +1,28 @@
 use oxc_ast::{
-    ast::{Class, ClassElement, Declaration, Function, ModuleDeclarationKind, PropertyDefinition},
+    ast::{Class, ClassElement, Function, PropertyDefinition},
     AstKind, Span,
 };
 use oxc_diagnostics::{
     miette::{self, Diagnostic},
     thiserror::Error,
 };
 use oxc_macros::declare_oxc_lint;
+use oxc_semantic::Symbol;
 
-use crate::{ast_util::IsPrivate, context::LintContext, rule::Rule, AstNode};
+use crate::{ast_util::IsPrivate, context::LintContext, rule::Rule};
 
 #[derive(Debug, Error, Diagnostic)]
-#[error("")]
-#[diagnostic(severity(warning), help(""))]
-struct IsolatedDeclarationDiagnostic(#[label] pub Span);
+enum IsolatedDeclarationDiagnostic {
+    #[error("isolated-declaration: Requires type annotation on non-private property definition")]
+    #[diagnostic(severity(warning))]
+    Property(#[label] Span),
+    #[error("isolated-declaration: Requires type annotation on parameters of non-private method")]
+    #[diagnostic(severity(warning))]
+    FunctionParam(#[label] Span),
+    #[error("isolated-declaration: Requires return type annotation of non-private method")]
+    #[diagnostic(severity(warning))]
+    FunctionReturnType(#[label] Span),
+}
 
 #[derive(Debug, Default, Clone)]
 pub struct IsolatedDeclaration;
@@ -26,35 +35,40 @@ declare_oxc_lint!(
     /// The thread on isolated declaration is at `https://github.com/microsoft/TypeScript/issues/47947`
     ///
     /// ### Why is this bad?
-    ///
+    /// The restrictions allow .d.ts files to be generated based on one single .ts file, which improves the
+    /// efficiency and possible parallelism of the declaration emitting process. Furthermore, it prevents syntax
+    /// errors in one file from propagating to other dependent files.
     ///
     /// ### Example
-    /// ```javascript
+    /// ```typescript
+    /// export class Test {
+    ///   x // error under isolated declarations
+    ///   private y = 0; // no error, private field types are not serialized in declarations
+    ///   #z = 1;// no error, fields is not present in declarations
+    ///   constructor(x: number) {
+    ///     this.x = 1;
+    ///   }
+    ///   get a() { // error under isolated declarations
+    ///     return 1;
+    ///   }
+    ///   set a(value) { // error under isolated declarations
+    ///     this.x = 1;
+    ///   }
+    /// }
     /// ```
     IsolatedDeclaration,
-    correctness
+    nursery,
 );
 
 impl Rule for IsolatedDeclaration {
-    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
-        let AstKind::ModuleDeclaration(module) = node.get().kind() else { return; };
-        match &module.kind {
-            ModuleDeclarationKind::ImportDeclaration(_) => todo!(),
-            ModuleDeclarationKind::ExportAllDeclaration(_) => todo!(),
-            ModuleDeclarationKind::ExportDefaultDeclaration(_) => todo!(),
-            ModuleDeclarationKind::ExportNamedDeclaration(decl) => {
-                if let Some(decl) = &decl.declaration {
-                    match decl {
-                        Declaration::FunctionDeclaration(function) => {
-                            Self::check_function(function, ctx)
-                        }
-                        Declaration::ClassDeclaration(class) => Self::check_class(class, ctx),
-                        _ => (),
-                    }
-                }
+    fn run_on_symbol(&self, symbol: &Symbol, ctx: &LintContext<'_>) {
+        if symbol.is_export() {
+            let declaration = ctx.nodes()[symbol.declaration()];
+            match declaration.kind() {
+                AstKind::Class(class) => Self::check_class(class, ctx),
+                AstKind::Function(function) => Self::check_function(function, ctx),
+                _ => (),
             }
-            ModuleDeclarationKind::TSExportAssignment(_) => todo!(),
-            ModuleDeclarationKind::TSNamespaceExportDeclaration(_) => todo!(),
         }
     }
 }
@@ -67,17 +81,18 @@ impl IsolatedDeclaration {
         let parameters = &function.params.items;
         for param in parameters {
             if param.pattern.type_annotation.is_none() {
-                ctx.diagnostic(IsolatedDeclarationDiagnostic(param.span));
+                ctx.diagnostic(IsolatedDeclarationDiagnostic::FunctionParam(param.span));
             }
         }
         if function.return_type.is_none() {
-            ctx.diagnostic(IsolatedDeclarationDiagnostic(function.span));
+            ctx.diagnostic(IsolatedDeclarationDiagnostic::FunctionReturnType(function.span));
         }
     }
 
+    /// Checks that the property as a type annotation
     pub fn check_property_definition(property: &PropertyDefinition, ctx: &LintContext<'_>) {
         if property.type_annotation.is_none() {
-            ctx.diagnostic(IsolatedDeclarationDiagnostic(property.span));
+            ctx.diagnostic(IsolatedDeclarationDiagnostic::Property(property.span));
         }
     }
 
@@ -88,25 +103,25 @@ impl IsolatedDeclaration {
     pub fn check_class(class: &Class, ctx: &LintContext<'_>) {
         for element in &class.body.body {
             match element {
-                ClassElement::StaticBlock(_) => (),
                 ClassElement::MethodDefinition(method) => {
                     if !method.is_private() {
                         Self::check_function(&method.value, ctx);
                     }
                 }
                 ClassElement::PropertyDefinition(property) => {
                     if !property.is_private() && !property.key.is_private_identifier() {
-                        Self::check_property_definition(property, ctx)
+                        Self::check_property_definition(property, ctx);
                     }
                 }
-                ClassElement::AccessorProperty(_) => todo!(),
                 ClassElement::TSAbstractMethodDefinition(method) => {
                     Self::check_function(&method.method_definition.value, ctx);
                 }
                 ClassElement::TSAbstractPropertyDefinition(property) => {
                     Self::check_property_definition(&property.property_definition, ctx);
                 }
-                ClassElement::TSIndexSignature(_) => todo!(),
+                ClassElement::StaticBlock(_)
+                | ClassElement::AccessorProperty(_)
+                | ClassElement::TSIndexSignature(_) => (),
             }
         }
     }

crates/oxc_linter/src/snapshots/isolated_declaration.snap.new → crates/oxc_linter/src/snapshots/isolated_declaration.snap

@@ -1,69 +1,59 @@
 ---
 source: crates/oxc_linter/src/tester.rs
-assertion_line: 67
 expression: isolated_declaration
 ---
 
-  ⚠ 
+  ⚠ isolated-declaration: Requires type annotation on parameters of non-private method
    ╭─[isolated_declaration.tsx:1:1]
  1 │ export function foo(a) { return a; }
    ·                     ─
    ╰────
-  help: 
 
-  ⚠ 
+  ⚠ isolated-declaration: Requires return type annotation of non-private method
    ╭─[isolated_declaration.tsx:1:1]
  1 │ export function foo(a) { return a; }
    ·        ─────────────────────────────
    ╰────
-  help: 
 
-  ⚠ 
+  ⚠ isolated-declaration: Requires type annotation on non-private property definition
    ╭─[isolated_declaration.tsx:1:1]
  1 │ export class A { public a; }
    ·                  ─────────
    ╰────
-  help: 
 
-  ⚠ 
+  ⚠ isolated-declaration: Requires return type annotation of non-private method
    ╭─[isolated_declaration.tsx:1:1]
  1 │ export class A { foo() { return 0; } }
    ·                     ────────────────
    ╰────
-  help: 
 
-  ⚠ 
+  ⚠ isolated-declaration: Requires return type annotation of non-private method
    ╭─[isolated_declaration.tsx:1:1]
  1 │ export abstract class A { abstract foo() { return 0; } }
    ·                                       ────────────────
    ╰────
-  help: 
 
-  ⚠ 
+  ⚠ isolated-declaration: Requires type annotation on non-private property definition
    ╭─[isolated_declaration.tsx:1:1]
  1 │ export abstract class A { abstract a; }
    ·                           ───────────
    ╰────
-  help: 
 
-  ⚠ 
+  ⚠ isolated-declaration: Requires return type annotation of non-private method
    ╭─[isolated_declaration.tsx:1:1]
  1 │ export class A { get foo() { return 0; } }
    ·                         ────────────────
    ╰────
-  help: 
 
-  ⚠ 
+  ⚠ isolated-declaration: Requires type annotation on parameters of non-private method
    ╭─[isolated_declaration.tsx:1:1]
  1 │ export class A { set foo(val) { } }
    ·                          ───
    ╰────
-  help: 
 
-  ⚠ 
+  ⚠ isolated-declaration: Requires return type annotation of non-private method
    ╭─[isolated_declaration.tsx:1:1]
  1 │ export class A { set foo(val) { } }
    ·                         ─────────
    ╰────
-  help: 
 

crates/oxc_parser/src/ts/statement.rs

@@ -395,8 +395,12 @@ impl<'a> Parser<'a> {
                     self.parse_ts_declare_function(start_span, modifiers)
                         .map(Declaration::FunctionDeclaration)
                 } else if self.ts_enabled() {
-                    self.parse_ts_function_impl(start_span, FunctionKind::TSDeclaration, modifiers)
-                        .map(Declaration::FunctionDeclaration)
+                    self.parse_ts_function_impl(
+                        start_span,
+                        FunctionKind::Declaration { single_statement: true },
+                        modifiers,
+                    )
+                    .map(Declaration::FunctionDeclaration)
                 } else {
                     self.parse_function_impl(FunctionKind::Declaration { single_statement: true })
                         .map(Declaration::FunctionDeclaration)

crates/oxc_semantic/src/binder.rs

@@ -155,9 +155,7 @@ impl<'a> Binder for CatchClause<'a> {
             // unless CatchParameter is CatchParameter : BindingIdentifier
             if let BindingPatternKind::BindingIdentifier(ident) = &param.kind {
                 let includes = SymbolFlags::FunctionScopedVariable | SymbolFlags::CatchVariable;
-                // Overshadows declarations so redeclarator error is not reported here
-                let symbol_id = builder.symbols.create(ident.name.clone(), ident.span, includes);
-                builder.scope.current_scope_mut().variables.insert(ident.name.clone(), symbol_id);
+                builder.declare_shadow_symbol(&ident.name, ident.span, current_scope_id, includes);
             } else {
                 for ident in param.bound_names() {
                     builder.declare_symbol(

crates/oxc_semantic/src/builder.rs

@@ -30,6 +30,7 @@ pub struct SemanticBuilder<'a> {
     // states
     pub current_node_id: AstNodeId,
     pub current_node_flags: NodeFlags,
+    pub current_symbol_flags: SymbolFlags,
 
     // builders
     pub nodes: AstNodes<'a>,
@@ -58,6 +59,7 @@ impl<'a> SemanticBuilder<'a> {
             errors: vec![],
             current_node_id,
             current_node_flags: NodeFlags::empty(),
+            current_symbol_flags: SymbolFlags::empty(),
             nodes,
             scope,
             symbols: SymbolTable::default(),
@@ -147,7 +149,22 @@ impl<'a> SemanticBuilder<'a> {
         if let Some(symbol_id) = self.check_redeclaration(scope_id, name, span, excludes) {
             return symbol_id;
         }
-        let symbol_id = self.symbols.create(name.clone(), span, includes);
+        let includes = includes | self.current_symbol_flags;
+        let symbol_id = self.symbols.create(self.current_node_id, name.clone(), span, includes);
+        self.scope.scopes[scope_id].variables.insert(name.clone(), symbol_id);
+        symbol_id
+    }
+
+    /// Declares a `Symbol` for the node, shadowing previous declarations in the same scope.
+    pub fn declare_shadow_symbol(
+        &mut self,
+        name: &Atom,
+        span: Span,
+        scope_id: ScopeId,
+        includes: SymbolFlags,
+    ) -> SymbolId {
+        let includes = includes | self.current_symbol_flags;
+        let symbol_id = self.symbols.create(self.current_node_id, name.clone(), span, includes);
         self.scope.scopes[scope_id].variables.insert(name.clone(), symbol_id);
         symbol_id
     }
@@ -193,6 +210,7 @@ impl<'a> SemanticBuilder<'a> {
     fn enter_kind(&mut self, kind: AstKind<'a>) {
         match kind {
             AstKind::ModuleDeclaration(decl) => {
+                self.current_symbol_flags |= Self::symbol_flag_from_module_declaration(decl);
                 decl.bind(self);
             }
             AstKind::VariableDeclarator(decl) => {
@@ -233,6 +251,9 @@ impl<'a> SemanticBuilder<'a> {
             AstKind::Class(_) => {
                 self.current_node_flags -= NodeFlags::Class;
             }
+            AstKind::ModuleDeclaration(decl) => {
+                self.current_symbol_flags -= Self::symbol_flag_from_module_declaration(decl);
+            }
             _ => {}
         }
     }
@@ -267,4 +288,12 @@ impl<'a> SemanticBuilder<'a> {
             }
         }
     }
+
+    fn symbol_flag_from_module_declaration(module: &ModuleDeclaration) -> SymbolFlags {
+        if matches!(&module.kind, ModuleDeclarationKind::ImportDeclaration(_)) {
+            SymbolFlags::Import
+        } else {
+            SymbolFlags::Export
+        }
+    }
 }

crates/oxc_semantic/src/symbol/mod.rs

@@ -18,9 +18,12 @@ use crate::node::AstNodeId;
 #[derive(Debug)]
 pub struct Symbol {
     id: SymbolId,
+    /// Pointer to the AST Node where this symbol is declared
+    declaration: AstNodeId,
     name: Atom,
     span: Span,
     flags: SymbolFlags,
+    /// Pointers to the AST Node that references this symbol
     references: Vec<AstNodeId>,
 }
 
@@ -41,6 +44,10 @@ bitflags! {
         const BlockScopedVariable     = 1 << 1;
         /// A const variable (const)
         const ConstVariable           = 1 << 2;
+        /// Is this symbol inside an import declaration
+        const Import                  = 1 << 3;
+        /// Is this symbol inside an export declaration
+        const Export                  = 1 << 4;
         const Class                   = 1 << 5;
         const CatchVariable           = 1 << 6; // try {} catch(catch_variable) {}
 
@@ -61,8 +68,14 @@ bitflags! {
 
 impl Symbol {
     #[must_use]
-    pub fn new(id: SymbolId, name: Atom, span: Span, flags: SymbolFlags) -> Self {
-        Self { id, name, span, flags, references: vec![] }
+    pub fn new(
+        id: SymbolId,
+        declaration: AstNodeId,
+        name: Atom,
+        span: Span,
+        flags: SymbolFlags,
+    ) -> Self {
+        Self { id, declaration, name, span, flags, references: vec![] }
     }
 
     #[must_use]
@@ -101,6 +114,11 @@ impl Symbol {
         self.flags.contains(SymbolFlags::Class)
     }
 
+    #[must_use]
+    pub fn is_export(&self) -> bool {
+        self.flags.contains(SymbolFlags::Export)
+    }
+
     pub fn add_references(&mut self, new_references: &[Reference]) {
         self.references.extend(new_references.iter().map(|r| r.ast_node_id));
     }
@@ -109,4 +127,9 @@ impl Symbol {
     pub fn references(&self) -> &[AstNodeId] {
         &self.references
     }
+
+    #[must_use]
+    pub fn declaration(&self) -> AstNodeId {
+        self.declaration
+    }
 }