Skip to content

Commit

Permalink
Improve ambiguous import error and position fixes
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@mstoykov
mstoykov committed Jul 10, 2024
1 parent b36ecc6 commit 248e41e
Show file tree
Hide file tree
Showing 3 changed files with 171 additions and 82 deletions.
25 changes: 20 additions & 5 deletions compiler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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) {
Expand Down
222 changes: 148 additions & 74 deletions modules_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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())
}
}
6 changes: 3 additions & 3 deletions vm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 248e41e

Please sign in to comment.