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:
Don Isaac 2024-04-05 11:01:27 -04:00 committed by GitHub
parent aa63b6491f
commit 1ea24ea0a7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 199 additions and 6 deletions

View file

@ -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);
}

View file

@ -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();
}

View file

@ -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,

View file

@ -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 {