mirror of
https://github.com/danbulant/oxc
synced 2026-05-24 12:21:58 +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) => {
|
AstKind::Class(class) => {
|
||||||
self.current_node_flags |= NodeFlags::Class;
|
self.current_node_flags |= NodeFlags::Class;
|
||||||
class.bind(self);
|
class.bind(self);
|
||||||
|
self.remove_export_flag();
|
||||||
self.make_all_namespaces_valuelike();
|
self.make_all_namespaces_valuelike();
|
||||||
}
|
}
|
||||||
AstKind::ClassBody(body) => {
|
AstKind::ClassBody(body) => {
|
||||||
|
|
@ -1700,6 +1701,9 @@ impl<'a> SemanticBuilder<'a> {
|
||||||
AstKind::BindingRestElement(element) => {
|
AstKind::BindingRestElement(element) => {
|
||||||
element.bind(self);
|
element.bind(self);
|
||||||
}
|
}
|
||||||
|
AstKind::FormalParameters(_) => {
|
||||||
|
self.remove_export_flag();
|
||||||
|
}
|
||||||
AstKind::FormalParameter(param) => {
|
AstKind::FormalParameter(param) => {
|
||||||
param.bind(self);
|
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) {
|
fn add_current_node_id_to_current_scope(&mut self) {
|
||||||
self.scope.add_node_id(self.current_scope_id, self.current_node_id);
|
self.scope.add_node_id(self.current_scope_id, self.current_node_id);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,7 @@
|
||||||
mod util;
|
mod util;
|
||||||
|
|
||||||
|
use oxc_semantic::SymbolFlags;
|
||||||
|
use oxc_syntax::module_record::ExportExportName;
|
||||||
pub use util::SemanticTester;
|
pub use util::SemanticTester;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|
@ -29,3 +31,140 @@ fn test_exports() {
|
||||||
// FIXME: failing
|
// FIXME: failing
|
||||||
// test.has_some_symbol("defaultExport").is_exported().test();
|
// 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()`]
|
/// Checks if the resolved symbol contains all flags in `flags`, using [`SymbolFlags::contains()`]
|
||||||
pub fn contains_flags(mut self, flags: SymbolFlags) -> Self {
|
pub fn contains_flags(mut self, flags: SymbolFlags) -> Self {
|
||||||
self.test_result = match self.test_result {
|
self.test_result = match self.test_result {
|
||||||
|
|
@ -142,12 +147,46 @@ impl<'a> SymbolTester<'a> {
|
||||||
self.test_result = match self.test_result {
|
self.test_result = match self.test_result {
|
||||||
Ok(symbol_id) => {
|
Ok(symbol_id) => {
|
||||||
let binding = self.target_symbol_name.clone();
|
let binding = self.target_symbol_name.clone();
|
||||||
if self.semantic.module_record().exported_bindings.contains_key(binding.as_str())
|
let is_in_module_record =
|
||||||
&& self.semantic.scopes().get_root_binding(&binding) == Some(symbol_id)
|
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 {
|
} else {
|
||||||
Err(miette!("Expected {binding} to be exported."))
|
Ok(symbol_id)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
e => e,
|
e => e,
|
||||||
|
|
|
||||||
|
|
@ -128,11 +128,11 @@ impl NameSpan {
|
||||||
Self { name, span }
|
Self { name, span }
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn name(&self) -> &CompactStr {
|
pub const fn name(&self) -> &CompactStr {
|
||||||
&self.name
|
&self.name
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn span(&self) -> Span {
|
pub const fn span(&self) -> Span {
|
||||||
self.span
|
self.span
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -261,6 +261,13 @@ impl ExportLocalName {
|
||||||
pub fn is_null(&self) -> bool {
|
pub fn is_null(&self) -> bool {
|
||||||
matches!(self, Self::Null)
|
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 {
|
pub struct FunctionMeta {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue