Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
check for type/value mismatch after program scope exits
  • Loading branch information
DonIsaac committed Jun 29, 2024
commit d1914dc5a24bddef21c5d96ea814e2f750af407c
65 changes: 59 additions & 6 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl<'a> SemanticBuilder<'a> {
current_scope_id,
function_stack: vec![],
namespace_stack: vec![],
meaning_stack: vec![],
meaning_stack: vec![SymbolFlags::Value],
nodes: AstNodes::default(),
scope,
symbols: SymbolTable::default(),
Expand Down Expand Up @@ -163,6 +163,9 @@ impl<'a> SemanticBuilder<'a> {
program.scope_id.set(Some(scope_id));
} else {
self.visit_program(program);
if self.check_syntax_error {
checker::check_last(&self);
}

// Checking syntax error on module record requires scope information from the previous AST pass
if self.check_syntax_error {
Expand Down Expand Up @@ -237,6 +240,14 @@ impl<'a> SemanticBuilder<'a> {
}
}

pub fn current_meaning(&self) -> SymbolFlags {
let meaning = self.meaning_stack.last().copied();
#[cfg(debug_assertions)]
return meaning.unwrap();
#[cfg(not(debug_assertions))]
return meaning.unwrap_or(SymbolFlags::all());
}

pub fn current_scope_flags(&self) -> ScopeFlags {
self.scope.get_flags(self.current_scope_id)
}
Expand Down Expand Up @@ -310,7 +321,7 @@ impl<'a> SemanticBuilder<'a> {
&mut self,
reference: Reference,
add_unresolved_reference: bool,
meaning: SymbolFlags
meaning: SymbolFlags,
) -> ReferenceId {
let reference_name = reference.name().clone();
let reference_id = self.symbols.create_reference(reference);
Expand All @@ -319,7 +330,7 @@ impl<'a> SemanticBuilder<'a> {
self.current_scope_id,
reference_name,
reference_id,
meaning
meaning,
);
} else {
self.resolve_reference_ids(reference_name.clone(), vec![(reference_id, meaning)]);
Expand Down Expand Up @@ -356,19 +367,27 @@ impl<'a> SemanticBuilder<'a> {
}
}

fn resolve_reference_ids(&mut self, name: CompactStr, reference_ids: Vec<(ReferenceId, SymbolFlags)>) {
fn resolve_reference_ids(
&mut self,
name: CompactStr,
reference_ids: Vec<(ReferenceId, SymbolFlags)>,
) {
let parent_scope_id =
self.scope.get_parent_id(self.current_scope_id).unwrap_or(self.current_scope_id);

if let Some(symbol_id) = self.scope.get_binding(self.current_scope_id, &name) {
let symbol_flags = self.symbols.get_flag(symbol_id);
let mut unresolved: Vec<(ReferenceId, SymbolFlags)> = Vec::with_capacity(reference_ids.len());
let mut unresolved: Vec<(ReferenceId, SymbolFlags)> =
Vec::with_capacity(reference_ids.len());
for (reference_id, meaning) in reference_ids {
let reference = &mut self.symbols.references[reference_id];
// if dbg!(symbol_flags).intersects(dbg!(meaning)) {
if symbol_flags.intersects(meaning) {
// println!("true");
reference.set_symbol_id(symbol_id);
self.symbols.resolved_references[symbol_id].push(reference_id);
} else {
// println!("false");
unresolved.push((reference_id, meaning))
}
}
Expand Down Expand Up @@ -1746,9 +1765,11 @@ impl<'a> SemanticBuilder<'a> {
self.namespace_stack.push(*symbol_id.unwrap());
}
AstKind::TSTypeAliasDeclaration(type_alias_declaration) => {
self.meaning_stack.push(SymbolFlags::Type);
type_alias_declaration.bind(self);
}
AstKind::TSInterfaceDeclaration(interface_declaration) => {
self.meaning_stack.push(SymbolFlags::Type);
interface_declaration.bind(self);
}
AstKind::TSEnumDeclaration(enum_declaration) => {
Expand All @@ -1760,14 +1781,24 @@ impl<'a> SemanticBuilder<'a> {
enum_member.bind(self);
}
AstKind::TSTypeParameter(type_parameter) => {
self.meaning_stack.push(SymbolFlags::Type);
type_parameter.bind(self);
}
AstKind::ExportSpecifier(s) if s.export_kind.is_type() => {
self.current_reference_flag = ReferenceFlag::Type;
}
AstKind::TSTypeName(_) => {
self.meaning_stack.push(SymbolFlags::Type);
self.current_reference_flag = ReferenceFlag::Type;
}
AstKind::TSTypeQuery(_) => {
// checks types of a value symbol (e.g. `typeof x`), so we're
// looking for a value even though its used as a type
self.meaning_stack.push(SymbolFlags::Value)
}
AstKind::TSTypeAnnotation(_) => {
self.meaning_stack.push(SymbolFlags::Type);
}
AstKind::IdentifierReference(ident) => {
self.reference_identifier(ident);
}
Expand Down Expand Up @@ -1803,6 +1834,7 @@ impl<'a> SemanticBuilder<'a> {
}
}
AstKind::YieldExpression(_) => {
self.meaning_stack.push(SymbolFlags::Value);
self.set_function_node_flag(NodeFlags::HasYield);
}
_ => {}
Expand Down Expand Up @@ -1851,9 +1883,25 @@ impl<'a> SemanticBuilder<'a> {
AstKind::TSModuleBlock(_) => {
self.namespace_stack.pop();
}
AstKind::TSTypeAliasDeclaration(_) => {
self.meaning_stack.pop();
}
AstKind::TSInterfaceDeclaration(_) => {
self.meaning_stack.pop();
}
AstKind::TSTypeParameter(_) => {
self.meaning_stack.pop();
}
AstKind::TSTypeName(_) => {
self.meaning_stack.pop();
self.current_reference_flag -= ReferenceFlag::Type;
}
AstKind::TSTypeQuery(_) => {
self.meaning_stack.pop();
}
AstKind::TSTypeAnnotation(_) => {
self.meaning_stack.pop();
}
AstKind::UpdateExpression(_) => {
if self.is_not_expression_statement_parent() {
self.current_reference_flag -= ReferenceFlag::Read;
Expand All @@ -1869,6 +1917,9 @@ impl<'a> SemanticBuilder<'a> {
}
AstKind::MemberExpression(_) => self.current_reference_flag = ReferenceFlag::empty(),
AstKind::AssignmentTarget(_) => self.current_reference_flag -= ReferenceFlag::Write,
AstKind::YieldExpression(_) => {
self.meaning_stack.pop();
}
_ => {}
}
}
Expand All @@ -1895,7 +1946,9 @@ impl<'a> SemanticBuilder<'a> {
// ^^^^^^^^^^^^^^^^^^^^ Parameter initializer must be resolved immediately
// to avoid binding to variables inside the scope
let add_unresolved_reference = !self.current_node_flags.has_parameter();
let reference_id = self.declare_reference(reference, add_unresolved_reference, SymbolFlags::Value);
let reference_id =
self.declare_reference(reference, add_unresolved_reference, self.current_meaning());
// self.declare_reference(reference, add_unresolved_reference, dbg!(self.current_meaning()));
ident.reference_id.set(Some(reference_id));
}

Expand Down
4 changes: 3 additions & 1 deletion crates/oxc_semantic/src/checker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ use typescript as ts;

use crate::{builder::SemanticBuilder, AstNode};

pub fn check_last<'a>(ctx: &SemanticBuilder<'a>) {
ts::check_symbol_resolution_failures(ctx);
}
pub fn check<'a>(node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
let kind = node.kind();

match kind {
AstKind::Program(_) => {
js::check_labeled_statement(ctx);
js::check_duplicate_class_elements(ctx);
// ts::check_symbol_resolution_failures(ctx);
}
AstKind::BindingIdentifier(ident) => {
js::check_identifier(&ident.name, ident.span, node, ctx);
Expand Down
54 changes: 30 additions & 24 deletions crates/oxc_semantic/src/checker/typescript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,36 @@ fn type_used_as_value(span: Span, name: &str) -> OxcDiagnostic {
.with_label(span)
}

// pub fn check_symbol_resolution_failures<'a>(ctx: &SemanticBuilder<'a>) {
// for (scope_id, name, reference_ids) in ctx.scope.iter_unresolved_references() {
// if let Some(symbol_id) = ctx.scope.find_binding(scope_id, name) {
// let symbol_flags = ctx.symbols.get_flag(symbol_id);
// for reference_id in reference_ids {
// let reference = ctx.symbols.get_reference(*reference_id);
// if reference.is_agnostic() {
// continue;
// }
// match (symbol_flags.is_type(), reference.is_type()) {
// (true, false) => {
// // checker.ts, checkAndReportErrorForUsingValueAsType
// ctx.error(value_used_as_type(reference.span(), name));
// }
// (false, true) => {
// // checker.ts, checkAndReportErrorForUsingTypeAsValue
// ctx.error(type_used_as_value(reference.span(), name));
// }
// _ => {}
// }
// }
// }
// }
// }
pub fn check_symbol_resolution_failures<'a>(ctx: &SemanticBuilder<'a>) {
for (scope_id, name, reference_ids) in ctx.scope.iter_unresolved_references() {
if let Some(symbol_id) = ctx.scope.find_binding(scope_id, name) {
let symbol_flags = ctx.symbols.get_flag(symbol_id);
for (reference_id, meaning) in reference_ids {
let reference = ctx.symbols.get_reference(*reference_id);
// note: is_value() does not imply !is_type()
if meaning.is_value() && symbol_flags.is_type() {

// checker.ts, checkAndReportErrorForUsingTypeAsValue
ctx.error(type_used_as_value(reference.span(), name));
} else if meaning.is_type() && symbol_flags.is_value() {
// checker.ts, checkAndReportErrorForUsingValueAsType
ctx.error(value_used_as_type(reference.span(), name));
}
// match (symbol_flags.is_type(), meaning.is_type()) {
// (true, false) => {
// // checker.ts, checkAndReportErrorForUsingValueAsType
// ctx.error(value_used_as_type(reference.span(), name));
// }
// (false, true) => {
// // checker.ts, checkAndReportErrorForUsingTypeAsValue
// ctx.error(type_used_as_value(reference.span(), name));
// }
// _ => {}
// }
}
}
}
}

fn empty_type_parameter_list(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::error("Type parameter list cannot be empty.").with_label(span0)
Expand Down
8 changes: 8 additions & 0 deletions crates/oxc_semantic/tests/integration/symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,11 @@ 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 foo() {
SemanticTester::ts("
type T = number;
type U = T;
").has_root_symbol("T").test();
}
7 changes: 6 additions & 1 deletion crates/oxc_syntax/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,13 @@ impl SymbolFlags {
self.intersects(Self::Variable)
}

pub fn is_value(&self) -> bool {
self.intersects(Self::Value)
}

/// Note: Some values can be types (e.g. classes, enums)
pub fn is_type(&self) -> bool {
!self.intersects(Self::Value)
self.intersects(Self::Type)
}

pub fn is_const_variable(&self) -> bool {
Expand Down
Loading