feat(semantic): check for illegal symbol modifiers (#3838)

crates/oxc_ast/src/ast/ts.rs

@@ -1339,7 +1339,16 @@ impl<'a> Modifiers<'a> {
             .map_or(false, |modifiers| modifiers.iter().any(|modifier| modifier.kind == target))
     }
 
-    pub fn find<F>(&self, f: F) -> Option<&Modifier>
+    pub fn iter(&self) -> impl Iterator<Item = &Modifier> + '_ {
+        self.0.as_ref().into_iter().flat_map(|modifiers| modifiers.iter())
+    }
+
+    /// Find a modifier by kind
+    pub fn find(&self, kind: ModifierKind) -> Option<&Modifier> {
+        self.find_where(|modifier| modifier.kind == kind)
+    }
+
+    pub fn find_where<F>(&self, f: F) -> Option<&Modifier>
     where
         F: Fn(&Modifier) -> bool,
     {

crates/oxc_linter/src/rules/oxc/no_const_enum.rs

@@ -41,8 +41,9 @@ declare_oxc_lint!(
 impl Rule for NoConstEnum {
     fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
         if let AstKind::TSEnumDeclaration(enum_decl) = node.kind() {
-            let Some(const_enum) =
-                enum_decl.modifiers.find(|modifier| matches!(modifier.kind, ModifierKind::Const))
+            let Some(const_enum) = enum_decl
+                .modifiers
+                .find_where(|modifier| matches!(modifier.kind, ModifierKind::Const))
             else {
                 return;
             };

crates/oxc_semantic/src/checker/mod.rs

@@ -34,6 +34,7 @@ pub fn check<'a>(node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
         AstKind::RegExpLiteral(lit) => js::check_regexp_literal(lit, ctx),
 
         AstKind::Directive(dir) => js::check_directive(dir, ctx),
+        AstKind::Function(func) => ts::check_function(func, node, ctx),
         AstKind::ModuleDeclaration(decl) => {
             js::check_module_declaration(decl, node, ctx);
         }
@@ -68,8 +69,10 @@ pub fn check<'a>(node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
                 js::check_function_declaration(alternate, true, ctx);
             }
         }
-
-        AstKind::Class(class) => js::check_class(class, node, ctx),
+        AstKind::Class(class) => {
+            js::check_class(class, node, ctx);
+            ts::check_class(class, node, ctx);
+        }
         AstKind::MethodDefinition(method) => js::check_method_definition(method, ctx),
         AstKind::ObjectProperty(prop) => js::check_object_property(prop, ctx),
         AstKind::Super(sup) => js::check_super(sup, node, ctx),
@@ -91,13 +94,20 @@ pub fn check<'a>(node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
         AstKind::ObjectExpression(expr) => js::check_object_expression(expr, ctx),
         AstKind::UnaryExpression(expr) => js::check_unary_expression(expr, node, ctx),
         AstKind::YieldExpression(expr) => js::check_yield_expression(expr, node, ctx),
+        AstKind::VariableDeclaration(decl) => ts::check_variable_declaration(decl, node, ctx),
         AstKind::VariableDeclarator(decl) => ts::check_variable_declarator(decl, ctx),
         AstKind::SimpleAssignmentTarget(target) => ts::check_simple_assignment_target(target, ctx),
         AstKind::TSTypeParameterDeclaration(declaration) => {
             ts::check_ts_type_parameter_declaration(declaration, ctx);
         }
-        AstKind::TSModuleDeclaration(decl) => ts::check_ts_module_declaration(decl, ctx),
-        AstKind::TSEnumDeclaration(decl) => ts::check_ts_enum_declaration(decl, ctx),
+        AstKind::TSModuleDeclaration(decl) => ts::check_ts_module_declaration(decl, node, ctx),
+        AstKind::TSEnumDeclaration(decl) => ts::check_ts_enum_declaration(decl, node, ctx),
+        AstKind::TSTypeAliasDeclaration(decl) => {
+            ts::check_ts_type_alias_declaration(decl, node, ctx);
+        }
+        AstKind::TSInterfaceDeclaration(decl) => {
+            ts::check_ts_interface_declaration(decl, node, ctx);
+        }
         _ => {}
     }
 }

crates/oxc_semantic/src/checker/typescript.rs

@@ -3,9 +3,10 @@ use oxc_ast::syntax_directed_operations::BoundNames;
 use oxc_ast::{ast::*, AstKind};
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_span::{Atom, GetSpan, Span};
+use oxc_syntax::scope::ScopeFlags;
 use rustc_hash::FxHashMap;
 
-use crate::{builder::SemanticBuilder, diagnostics::redeclaration};
+use crate::{builder::SemanticBuilder, diagnostics::redeclaration, AstNode};
 
 fn empty_type_parameter_list(span0: Span) -> OxcDiagnostic {
     OxcDiagnostic::error("Type parameter list cannot be empty.").with_labels([span0.into()])
@@ -23,7 +24,6 @@ pub fn check_ts_type_parameter_declaration(
 fn unexpected_optional(span0: Span) -> OxcDiagnostic {
     OxcDiagnostic::error("Unexpected `?` operator").with_labels([span0.into()])
 }
-
 #[allow(clippy::cast_possible_truncation)]
 pub fn check_variable_declarator(decl: &VariableDeclarator, ctx: &SemanticBuilder<'_>) {
     if decl.id.optional {
@@ -114,14 +114,107 @@ pub fn check_array_pattern<'a>(pattern: &ArrayPattern<'a>, ctx: &SemanticBuilder
     }
 }
 
+/// ts(1184)
+fn modifiers_cannot_be_used_here(span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::error("Modifiers cannot be used here.").with_labels([span.into()])
+}
+
+/// ts(1024)
+fn readonly_only_on_index_signature_or_property_decl(span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::error(
+        "'readonly' modifier can only appear on a property declaration or index signature.",
+    )
+    .with_labels([span.into()])
+}
+/// ts(1042)
+fn async_cannot_be_used_here(span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::error("'async' modifier cannot be used here.").with_labels([span.into()])
+}
+
+/// Returns `true` if the scope described by `scope_flags` supports export
+/// declarations or specifiers.
+pub const fn scope_can_export(scope_flags: ScopeFlags) -> bool {
+    const CAN_EXPORT: ScopeFlags = ScopeFlags::Top.union(ScopeFlags::TsModuleBlock);
+    scope_flags.contains(CAN_EXPORT)
+}
+
+fn check_declaration_modifiers<'a>(
+    modifiers: &Modifiers<'a>,
+    decl_node: &AstNode<'a>,
+    ctx: &SemanticBuilder<'a>,
+) {
+    let scope_flags = ctx.scope.get_flags(decl_node.scope_id());
+    let kind = decl_node.kind();
+    let is_class = matches!(kind, AstKind::Class(_));
+    let is_function = matches!(kind, AstKind::Function(_));
+    for modifier in modifiers.iter() {
+        match modifier.kind {
+            ModifierKind::Public
+            | ModifierKind::Private
+            | ModifierKind::Protected
+            | ModifierKind::Static
+            | ModifierKind::Override => {
+                ctx.error(modifiers_cannot_be_used_here(modifier.span));
+            }
+            ModifierKind::Abstract if !is_class => {
+                ctx.error(modifiers_cannot_be_used_here(modifier.span));
+            }
+            ModifierKind::Async if !is_function => {
+                ctx.error(async_cannot_be_used_here(modifier.span));
+            }
+            ModifierKind::Readonly => {
+                ctx.error(readonly_only_on_index_signature_or_property_decl(modifier.span));
+            }
+            ModifierKind::Export if !scope_can_export(scope_flags) => {
+                ctx.error(modifiers_cannot_be_used_here(modifier.span));
+            }
+            _ => {}
+        }
+    }
+}
+pub fn check_variable_declaration<'a>(
+    decl: &VariableDeclaration<'a>,
+    node: &AstNode<'a>,
+    ctx: &SemanticBuilder<'a>,
+) {
+    check_declaration_modifiers(&decl.modifiers, node, ctx);
+}
+
+pub fn check_function<'a>(function: &Function<'a>, node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
+    check_declaration_modifiers(&function.modifiers, node, ctx);
+}
+pub fn check_class<'a>(class: &Class<'a>, node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
+    check_declaration_modifiers(&class.modifiers, node, ctx);
+}
+
+pub fn check_ts_type_alias_declaration<'a>(
+    decl: &TSTypeAliasDeclaration<'a>,
+    node: &AstNode<'a>,
+    ctx: &SemanticBuilder<'a>,
+) {
+    check_declaration_modifiers(&decl.modifiers, node, ctx);
+}
+pub fn check_ts_interface_declaration<'a>(
+    decl: &TSInterfaceDeclaration<'a>,
+    node: &AstNode<'a>,
+    ctx: &SemanticBuilder<'a>,
+) {
+    check_declaration_modifiers(&decl.modifiers, node, ctx);
+}
+
 fn not_allowed_namespace_declaration(span0: Span) -> OxcDiagnostic {
     OxcDiagnostic::error(
         "A namespace declaration is only allowed at the top level of a namespace or module.",
     )
     .with_labels([span0.into()])
 }
 
-pub fn check_ts_module_declaration<'a>(decl: &TSModuleDeclaration<'a>, ctx: &SemanticBuilder<'a>) {
+pub fn check_ts_module_declaration<'a>(
+    decl: &TSModuleDeclaration<'a>,
+    node: &AstNode<'a>,
+    ctx: &SemanticBuilder<'a>,
+) {
+    check_declaration_modifiers(&decl.modifiers, node, ctx);
     // skip current node
     for node in ctx.nodes.iter_parents(ctx.current_node_id).skip(1) {
         match node.kind() {
@@ -144,8 +237,13 @@ fn enum_member_must_have_initializer(span0: Span) -> OxcDiagnostic {
     OxcDiagnostic::error("Enum member must have initializer.").with_labels([span0.into()])
 }
 
-pub fn check_ts_enum_declaration(decl: &TSEnumDeclaration<'_>, ctx: &SemanticBuilder<'_>) {
+pub fn check_ts_enum_declaration<'a>(
+    decl: &TSEnumDeclaration<'a>,
+    node: &AstNode<'a>,
+    ctx: &SemanticBuilder<'a>,
+) {
     let mut need_initializer = false;
+    check_declaration_modifiers(&decl.modifiers, node, ctx);
 
     decl.members.iter().for_each(|member| {
         #[allow(clippy::unnested_or_patterns)]

crates/oxc_semantic/tests/integration/modules.rs

@@ -1,4 +1,4 @@
-use oxc_semantic::SymbolFlags;
+use oxc_semantic::{SemanticBuilderReturn, SymbolFlags};
 
 use crate::util::SemanticTester;
 
@@ -46,6 +46,23 @@ fn test_exported_named_function() {
         .has_some_symbol("T")
         .is_not_exported()
         .test();
+
+    SemanticTester::tsx(
+        "
+    import React from 'react';
+    export const Counter: React.FC<{ count: number }> = ({ count }) => (
+        <div>{count}</div>
+    )
+    ",
+    )
+    .has_some_symbol("Counter")
+    .is_exported()
+    .contains_flags(
+        SymbolFlags::ConstVariable
+            .union(SymbolFlags::BlockScopedVariable)
+            .union(SymbolFlags::Export),
+    )
+    .test();
 }
 
 #[test]
@@ -149,3 +166,39 @@ fn test_exported_interface() {
     test.has_some_symbol("a").is_not_exported().test();
     test.has_some_symbol("T").is_not_exported().test();
 }
+
+#[test]
+fn test_exports_in_namespace() {
+    let test = SemanticTester::ts(
+        "
+    export const x = 1;
+    namespace N {
+        function foo() {
+            return 1
+        }
+        export function bar() {
+            return foo();
+        }
+        export const x = 2
+    } 
+    ",
+    );
+    test.has_some_symbol("bar").is_exported().test();
+    let semantic = test.build();
+    assert!(!semantic.module_record().exported_bindings.contains_key("bar"));
+}
+
+#[test]
+fn test_export_in_invalid_scope() {
+    let test = SemanticTester::js(
+        "
+    function foo() {
+        export const x = 1;
+    }",
+    )
+    .expect_errors(true);
+    test.has_some_symbol("x").contains_flags(SymbolFlags::Export).test();
+    let SemanticBuilderReturn { semantic, errors } = test.build_with_errors();
+    assert!(!errors.is_empty(), "expected an export within a function to produce a check error, but no errors were produced");
+    assert!(semantic.module_record().exported_bindings.is_empty());
+}

crates/oxc_semantic/tests/integration/symbols.rs

@@ -114,3 +114,20 @@ fn test_export_flag() {
     tester.has_root_symbol("b").contains_flags(SymbolFlags::Export).test();
     tester.has_root_symbol("c").contains_flags(SymbolFlags::Export).test();
 }
+
+#[test]
+fn test_invalid_modifiers() {
+    const PARAM_PROPERTY: &str =
+        "A parameter property is only allowed in a constructor implementation.";
+    const ILLEGAL_MODIFIER: &str = "Modifiers cannot be used here.";
+    const READONLY: &str =
+        "'readonly' modifier can only appear on a property declaration or index signature.";
+
+    SemanticTester::ts("function foo(public x: number) { }").has_error(PARAM_PROPERTY);
+    // SemanticTester::ts("function foo() { export const x = 1; }").has_error(illegal_modifier);
+    SemanticTester::ts("function foo() { public const x = 1; }").has_error(ILLEGAL_MODIFIER);
+    SemanticTester::ts("function foo() { private const x = 1; }").has_error(ILLEGAL_MODIFIER);
+    SemanticTester::ts("function foo() { protected const x = 1; }").has_error(ILLEGAL_MODIFIER);
+    SemanticTester::ts("function foo() { abstract const x = 1; }").has_error(ILLEGAL_MODIFIER);
+    SemanticTester::ts("function foo() { readonly const x = 1; }").has_error(READONLY);
+}