From 7f11ba3d5d90f4d1218069d3f9ad76d9b3c9f444 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 10 Jul 2024 17:21:05 +0300 Subject: [PATCH] Improve ambiguous import error and position fixes This splits the errors to distinguish between ambiguous imports - ones where multiple of the same identifiers were exported, and we can't know which it import. And the case where there is just no such export when it is imported. Also fixes line number. The source file is still listed twice. Fixes #11 --- compiler.go | 25 ++++-- modules_test.go | 222 ++++++++++++++++++++++++++++++++---------------- vm.go | 6 +- 3 files changed, 171 insertions(+), 82 deletions(-) diff --git a/compiler.go b/compiler.go index 946a67b..7fbd91b 100644 --- a/compiler.go +++ b/compiler.go @@ -977,7 +977,7 @@ func (c *compiler) compileModule(module *SourceTextModuleRecord) { for _, in := range module.indirectExportEntries { v, ambiguous := module.ResolveExport(in.exportName) if v == nil || ambiguous { - c.compileAmbiguousImport(unistring.NewFromString(in.importName)) + c.compileImportError(ambiguous, in.moduleRequest, in.exportName, in.offset) } } @@ -1057,7 +1057,7 @@ func (c *compiler) compileImportEntry(in importEntry) { if in.importName != "*" { resolution, ambiguous := importedModule.ResolveExport(in.importName) if resolution == nil || ambiguous { - c.compileAmbiguousImport(unistring.NewFromString(in.importName)) + c.compileImportError(ambiguous, in.moduleRequest, in.importName, in.offset) return } } @@ -1102,7 +1102,7 @@ func (c *compiler) compileIndirectExportEntry(entry exportEntry) { } b, ambiguous := otherModule.ResolveExport(entry.importName) if ambiguous || b == nil { - c.compileAmbiguousImport(unistring.NewFromString(entry.importName)) + c.compileImportError(ambiguous, entry.moduleRequest, entry.importName, entry.offset) return } @@ -1213,8 +1213,23 @@ func (c *compiler) compile(in *ast.Program, strict, inGlobal bool, evalVm *vm) { c.stringCache = nil } -func (c *compiler) compileAmbiguousImport(name unistring.String) { - c.emit(ambiguousImport(name)) +func (c *compiler) compileImportError(ambiguous bool, module, identifier string, offset int) { + if ambiguous { + c.compileSyntaxError( + fmt.Sprintf("The requested module %q contains conflicting star exports for name %q", + module, identifier), + offset) + } else { + c.compileSyntaxError( + fmt.Sprintf("The requested module %q does not provide an export named %q", + module, identifier), + offset) + } +} + +func (c *compiler) compileSyntaxError(msg string, offset int) { + c.p.addSrcMap(offset) + c.emit(runtimeSyntaxError(msg)) } func (c *compiler) compileDeclList(v []*ast.VariableDeclaration, inFunc bool) { diff --git a/modules_test.go b/modules_test.go index a97e05e..a4d990f 100644 --- a/modules_test.go +++ b/modules_test.go @@ -232,88 +232,16 @@ func TestSimpleModule(t *testing.T) { cases := cases t.Run(name, func(t *testing.T) { t.Parallel() - mu := sync.Mutex{} - cache := make(map[string]cacheElement) - var hostResolveImportedModule func(referencingScriptOrModule interface{}, specifier string) (ModuleRecord, error) - hostResolveImportedModule = func(referencingScriptOrModule interface{}, specifier string) (ModuleRecord, error) { - mu.Lock() - defer mu.Unlock() - k, ok := cache[specifier] - if ok { - return k.m, k.err - } - - src := string(cases[specifier]) - p, err := ParseModule(specifier, src, hostResolveImportedModule) - if err != nil { - cache[specifier] = cacheElement{err: err} - return nil, err - } - cache[specifier] = cacheElement{m: p} - return p, nil - } - - linked := make(map[ModuleRecord]error) - linkMu := new(sync.Mutex) - link := func(m ModuleRecord) error { - linkMu.Lock() - defer linkMu.Unlock() - if err, ok := linked[m]; ok { - return err - } - err := m.Link() - linked[m] = err - return err - } - - m, err := hostResolveImportedModule(nil, "a.js") - if err != nil { - t.Fatalf("got error %s", err) - } - p := m.(*SourceTextModuleRecord) - - err = link(p) - if err != nil { - t.Fatalf("got error %s", err) - } + fn := runModules(t, cases) for i := 0; i < 10; i++ { i := i - m := m t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { t.Parallel() vm := New() - eventLoopQueue := make(chan func(), 2) // the most basic and likely buggy event loop - vm.SetImportModuleDynamically(func(referencingScriptOrModule interface{}, specifierValue Value, pcap interface{}) { - specifier := specifierValue.String() - - eventLoopQueue <- func() { - ex := vm.runWrapped(func() { - m, err := hostResolveImportedModule(referencingScriptOrModule, specifier) - vm.FinishLoadingImportModule(referencingScriptOrModule, specifierValue, pcap, m, err) - }) - if ex != nil { - vm.FinishLoadingImportModule(referencingScriptOrModule, specifierValue, pcap, nil, ex) - } - } - }) - var promise *Promise - eventLoopQueue <- func() { promise = m.Evaluate(vm) } - - outer: - for { - select { - case fn := <-eventLoopQueue: - fn() - default: - break outer - } - } - + promise := fn(vm) if promise.state != PromiseStateFulfilled { t.Fatalf("got %+v", promise.Result().Export()) - err := promise.Result().Export().(error) - t.Fatalf("got error %s", err) } v := vm.Get("s") if v == nil || v.ToNumber().ToInteger() != 5 { @@ -324,3 +252,149 @@ func TestSimpleModule(t *testing.T) { }) } } + +func runModules(t testing.TB, files map[string]string) func(*Runtime) *Promise { + type cacheElement struct { + m ModuleRecord + err error + } + mu := sync.Mutex{} + cache := make(map[string]cacheElement) + var hostResolveImportedModule func(referencingScriptOrModule interface{}, specifier string) (ModuleRecord, error) + hostResolveImportedModule = func(_ interface{}, specifier string) (ModuleRecord, error) { + mu.Lock() + defer mu.Unlock() + k, ok := cache[specifier] + if ok { + return k.m, k.err + } + + src, ok := files[specifier] + if !ok { + return nil, fmt.Errorf("can't find %q from files", specifier) + } + p, err := ParseModule(specifier, src, hostResolveImportedModule) + if err != nil { + cache[specifier] = cacheElement{err: err} + return nil, err + } + cache[specifier] = cacheElement{m: p} + return p, nil + } + + linked := make(map[ModuleRecord]error) + linkMu := new(sync.Mutex) + link := func(m ModuleRecord) error { + linkMu.Lock() + defer linkMu.Unlock() + if err, ok := linked[m]; ok { + return err + } + err := m.Link() + linked[m] = err + return err + } + + m, err := hostResolveImportedModule(nil, "a.js") + if err != nil { + t.Fatalf("got error %s", err) + } + p := m.(*SourceTextModuleRecord) + + err = link(p) + if err != nil { + t.Fatalf("got error %s", err) + } + + return func(vm *Runtime) *Promise { + eventLoopQueue := make(chan func(), 2) // the most basic and likely buggy event loop + vm.SetImportModuleDynamically(func(referencingScriptOrModule interface{}, specifierValue Value, pcap interface{}) { + specifier := specifierValue.String() + + eventLoopQueue <- func() { + ex := vm.runWrapped(func() { + m, err := hostResolveImportedModule(referencingScriptOrModule, specifier) + vm.FinishLoadingImportModule(referencingScriptOrModule, specifierValue, pcap, m, err) + }) + if ex != nil { + vm.FinishLoadingImportModule(referencingScriptOrModule, specifierValue, pcap, nil, ex) + } + } + }) + var promise *Promise + eventLoopQueue <- func() { promise = m.Evaluate(vm) } + + outer: + for { + select { + case fn := <-eventLoopQueue: + fn() + default: + break outer + } + } + return promise + } +} + +func TestAmbiguousImport(t *testing.T) { + t.Parallel() + fn := runModules(t, map[string]string{ + `a.js`: ` + import "dep.js" + export let s = 5; + export * from "test1.js" + export * from "test2.js" + `, + `dep.js`: ` + import { s } from "a.js" + import { x } from "a.js" + `, + `test1.js`: ` + export let x = 6 + export let a = 6 + `, + `test2.js`: ` + export let x = 6 + export let b = 6 + `, + }) + promise := fn(New()) + if promise.state != PromiseStateRejected { + t.Fatalf("expected promise to be rejected %q", promise.state) + } + exc := promise.Result().Export().(*Exception) + expValue := `SyntaxError: The requested module "a.js" contains conflicting star exports for name "x" + at dep.js:3:5(2) + at dep.js:1:1(2) +` + if exc.String() != expValue { + t.Fatalf("Expected values %q but got %q", expValue, exc.String()) + } +} + +func TestImportingUnexported(t *testing.T) { + t.Parallel() + fn := runModules(t, map[string]string{ + `a.js`: ` + import "dep.js" + export let s = 5; + `, + `dep.js`: ` + import { s } from "a.js" + import { x } from "a.js" + `, + }) + promise := fn(New()) + if promise.state != PromiseStateRejected { + t.Fatalf("expected promise to be rejected %q", promise.state) + } + exc := promise.Result().Export().(*Exception) + expValue := `SyntaxError: The requested module "a.js" does not provide an export named "x" + at dep.js:3:5(2) + at dep.js:1:1(2) +` + if exc.String() != expValue { + t.Fatalf("Expected values %q but got %q", expValue, exc.String()) + } +} diff --git a/vm.go b/vm.go index d63bacf..6830913 100644 --- a/vm.go +++ b/vm.go @@ -3957,10 +3957,10 @@ func (n *newArrowFunc) _exec(vm *vm, obj *arrowFuncObject) { vm.pc++ } -type ambiguousImport unistring.String +type runtimeSyntaxError string -func (a ambiguousImport) exec(vm *vm) { - panic(vm.r.newError(vm.r.getSyntaxError(), "Ambiguous import for name %s", a)) +func (a runtimeSyntaxError) exec(vm *vm) { + panic(vm.r.newError(vm.r.getSyntaxError(), (string)(a))) } func (n *newArrowFunc) exec(vm *vm) {