mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 12:19:15 +00:00
fix(semantic): symbols inside functions and classes incorrectly flagged as exported (#2896)
# What This PR Does
Symbols declared inside exported functions and classes were being
incorrectly flagged with `SymbolFlags::Export`.
For example,
```ts
export function foo<T>(a: T) {
let b = String(a)
return b
}
```
`T`, `a`, and `b` were all flagged as exported.
## Further Work
It doesn't seem like exported enums and interfaces are being included in
`ModuleRecord`. Am I looking in the wrong place, or are they actually
missing?
This commit is contained in:
parent
aa63b6491f
commit
1ea24ea0a7
4 changed files with 199 additions and 6 deletions
|
|
@ -1681,6 +1681,7 @@ impl<'a> SemanticBuilder<'a> {
|
|||
AstKind::Class(class) => {
|
||||
self.current_node_flags |= NodeFlags::Class;
|
||||
class.bind(self);
|
||||
self.remove_export_flag();
|
||||
self.make_all_namespaces_valuelike();
|
||||
}
|
||||
AstKind::ClassBody(body) => {
|
||||
|
|
@ -1700,6 +1701,9 @@ impl<'a> SemanticBuilder<'a> {
|
|||
AstKind::BindingRestElement(element) => {
|
||||
element.bind(self);
|
||||
}
|
||||
AstKind::FormalParameters(_) => {
|
||||
self.remove_export_flag();
|
||||
}
|
||||
AstKind::FormalParameter(param) => {
|
||||
param.bind(self);
|
||||
}
|
||||
|
|
@ -1832,6 +1836,10 @@ impl<'a> SemanticBuilder<'a> {
|
|||
}
|
||||
}
|
||||
|
||||
fn remove_export_flag(&mut self) {
|
||||
self.current_symbol_flags -= SymbolFlags::Export;
|
||||
}
|
||||
|
||||
fn add_current_node_id_to_current_scope(&mut self) {
|
||||
self.scope.add_node_id(self.current_scope_id, self.current_node_id);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
mod util;
|
||||
|
||||
use oxc_semantic::SymbolFlags;
|
||||
use oxc_syntax::module_record::ExportExportName;
|
||||
pub use util::SemanticTester;
|
||||
|
||||
#[test]
|
||||
|
|
@ -29,3 +31,140 @@ fn test_exports() {
|
|||
// FIXME: failing
|
||||
// test.has_some_symbol("defaultExport").is_exported().test();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_exported_named_function() {
|
||||
let test = SemanticTester::js(
|
||||
"
|
||||
export function foo(a) {
|
||||
let x = 1;
|
||||
}
|
||||
",
|
||||
);
|
||||
test.has_some_symbol("foo").is_exported().test();
|
||||
for name in &["a", "x"] {
|
||||
test.has_some_symbol(name).is_not_exported().test();
|
||||
}
|
||||
|
||||
SemanticTester::ts("export function foo<T>(a: T) { a.length }")
|
||||
.has_some_symbol("T")
|
||||
.is_not_exported()
|
||||
.test();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_exported_default_function() {
|
||||
let test = SemanticTester::js(
|
||||
"
|
||||
export default function foo(a) {
|
||||
let x = 1;
|
||||
}
|
||||
",
|
||||
);
|
||||
for name in &["a", "x"] {
|
||||
test.has_some_symbol(name).is_not_exported().test();
|
||||
}
|
||||
|
||||
let test = SemanticTester::ts("export default function <T extends string>(a: T) { a.length }");
|
||||
test.has_some_symbol("a").is_not_exported().test();
|
||||
test.has_some_symbol("T").is_not_exported().test();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_exported_named_class() {
|
||||
let test = SemanticTester::ts(
|
||||
"
|
||||
export class Foo<T> {
|
||||
constructor(a) {
|
||||
this.a = a;
|
||||
}
|
||||
|
||||
bar() {
|
||||
return this.a;
|
||||
}
|
||||
}
|
||||
",
|
||||
);
|
||||
|
||||
test.has_class("Foo");
|
||||
test.has_some_symbol("Foo").is_exported().test();
|
||||
// NOTE: bar() is not a symbol. Should it be?
|
||||
for name in &["a", "T"] {
|
||||
test.has_some_symbol(name).is_not_exported().test();
|
||||
}
|
||||
|
||||
SemanticTester::ts(
|
||||
"
|
||||
class Foo {};
|
||||
export { Foo }
|
||||
",
|
||||
)
|
||||
.has_some_symbol("Foo")
|
||||
.is_exported()
|
||||
.test();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_exported_default_class() {
|
||||
let test = SemanticTester::ts(
|
||||
"
|
||||
export default class Foo<T> {
|
||||
constructor(a) {
|
||||
this.a = a;
|
||||
}
|
||||
}
|
||||
",
|
||||
);
|
||||
|
||||
test.has_class("Foo");
|
||||
test.has_some_symbol("a").is_not_exported().test();
|
||||
test.has_some_symbol("T").is_not_exported().test();
|
||||
|
||||
{
|
||||
let foo_test = test.has_some_symbol("Foo");
|
||||
let (semantic, _) = foo_test.inner();
|
||||
let m = semantic.module_record();
|
||||
let local_default_entry = m
|
||||
.local_export_entries
|
||||
.iter()
|
||||
.find(|export| matches!(export.export_name, ExportExportName::Default(_)))
|
||||
.unwrap();
|
||||
assert!(local_default_entry.local_name.name().is_some_and(|name| name == &"Foo"));
|
||||
assert!(!m.exported_bindings.contains_key("Foo"));
|
||||
assert!(m.export_default.is_some());
|
||||
foo_test.contains_flags(SymbolFlags::Export).test();
|
||||
}
|
||||
}
|
||||
|
||||
// FIXME
|
||||
#[test]
|
||||
#[ignore]
|
||||
fn test_exported_enum() {
|
||||
let test = SemanticTester::ts(
|
||||
"
|
||||
export enum Foo {
|
||||
A = 1,
|
||||
B,
|
||||
}
|
||||
",
|
||||
);
|
||||
test.has_some_symbol("Foo").is_exported().contains_flags(SymbolFlags::RegularEnum).test();
|
||||
test.has_some_symbol("A").is_not_exported().contains_flags(SymbolFlags::EnumMember).test();
|
||||
test.has_some_symbol("B").is_not_exported().contains_flags(SymbolFlags::EnumMember).test();
|
||||
}
|
||||
|
||||
// FIXME
|
||||
#[test]
|
||||
#[ignore]
|
||||
fn test_exported_interface() {
|
||||
let test = SemanticTester::ts(
|
||||
"
|
||||
export interface Foo<T> {
|
||||
a: T;
|
||||
}
|
||||
",
|
||||
);
|
||||
test.has_root_symbol("Foo").is_exported().contains_flags(SymbolFlags::Interface).test();
|
||||
test.has_some_symbol("a").is_not_exported().test();
|
||||
test.has_some_symbol("T").is_not_exported().test();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -59,6 +59,11 @@ impl<'a> SymbolTester<'a> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Get inner resources without consuming `self`
|
||||
pub fn inner(&self) -> (Rc<Semantic<'a>>, SymbolId) {
|
||||
(Rc::clone(&self.semantic), *self.test_result.as_ref().unwrap())
|
||||
}
|
||||
|
||||
/// Checks if the resolved symbol contains all flags in `flags`, using [`SymbolFlags::contains()`]
|
||||
pub fn contains_flags(mut self, flags: SymbolFlags) -> Self {
|
||||
self.test_result = match self.test_result {
|
||||
|
|
@ -142,12 +147,46 @@ impl<'a> SymbolTester<'a> {
|
|||
self.test_result = match self.test_result {
|
||||
Ok(symbol_id) => {
|
||||
let binding = self.target_symbol_name.clone();
|
||||
if self.semantic.module_record().exported_bindings.contains_key(binding.as_str())
|
||||
&& self.semantic.scopes().get_root_binding(&binding) == Some(symbol_id)
|
||||
let is_in_module_record =
|
||||
self.semantic.module_record().exported_bindings.contains_key(binding.as_str())
|
||||
&& self.semantic.scopes().get_root_binding(&binding) == Some(symbol_id);
|
||||
let has_export_flag = self.semantic.symbols().get_flag(symbol_id).is_export();
|
||||
match (is_in_module_record, has_export_flag) {
|
||||
(false, false) => Err(miette!(
|
||||
"Expected {binding} to be exported. Symbol is not in module record and does not have SymbolFlags::Export"
|
||||
)),
|
||||
(false, true) => Err(miette!(
|
||||
"Expected {binding} to be exported. Symbol is not in module record, but has SymbolFlags::Export"
|
||||
)),
|
||||
(true, false) => Err(miette!(
|
||||
"Expected {binding} to be exported. Symbol is in module record, but does not have SymbolFlags::Export"
|
||||
)),
|
||||
(true, true) => Ok(symbol_id)
|
||||
}
|
||||
}
|
||||
e => e,
|
||||
};
|
||||
self
|
||||
}
|
||||
|
||||
#[allow(clippy::wrong_self_convention)]
|
||||
pub fn is_not_exported(mut self) -> Self {
|
||||
self.test_result = match self.test_result {
|
||||
Ok(symbol_id) => {
|
||||
let binding = self.target_symbol_name.clone();
|
||||
if self.semantic.symbols().get_flag(symbol_id).contains(SymbolFlags::Export) {
|
||||
Err(miette!("Expected {binding} to not be exported. Symbol has export flag."))
|
||||
} else if self
|
||||
.semantic
|
||||
.module_record()
|
||||
.exported_bindings
|
||||
.contains_key(binding.as_str())
|
||||
{
|
||||
Ok(symbol_id)
|
||||
Err(miette!(
|
||||
"Expected {binding} to not be exported. Binding is in the module record"
|
||||
))
|
||||
} else {
|
||||
Err(miette!("Expected {binding} to be exported."))
|
||||
Ok(symbol_id)
|
||||
}
|
||||
}
|
||||
e => e,
|
||||
|
|
|
|||
|
|
@ -128,11 +128,11 @@ impl NameSpan {
|
|||
Self { name, span }
|
||||
}
|
||||
|
||||
pub fn name(&self) -> &CompactStr {
|
||||
pub const fn name(&self) -> &CompactStr {
|
||||
&self.name
|
||||
}
|
||||
|
||||
pub fn span(&self) -> Span {
|
||||
pub const fn span(&self) -> Span {
|
||||
self.span
|
||||
}
|
||||
}
|
||||
|
|
@ -261,6 +261,13 @@ impl ExportLocalName {
|
|||
pub fn is_null(&self) -> bool {
|
||||
matches!(self, Self::Null)
|
||||
}
|
||||
|
||||
pub const fn name(&self) -> Option<&CompactStr> {
|
||||
match self {
|
||||
Self::Name(name) => Some(name.name()),
|
||||
Self::Default(_) | Self::Null => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub struct FunctionMeta {
|
||||
|
|
|
|||
Loading…
Reference in a new issue