pass directory errors to callers (#348)

internal/fs/fs.go

@@ -90,7 +90,7 @@ func (e *Entry) stat() {
 type FS interface {
 	// The returned map is immutable and is cached across invocations. Do not
 	// mutate it.
-	ReadDirectory(path string) map[string]*Entry
+	ReadDirectory(path string) (map[string]*Entry, error)
 	ReadFile(path string) (string, error)
 
 	// This is part of the interface because the mock interface used for tests
@@ -143,8 +143,12 @@ func MockFS(input map[string]string) FS {
 	return &mockFS{dirs, files}
 }
 
-func (fs *mockFS) ReadDirectory(path string) map[string]*Entry {
-	return fs.dirs[path]
+func (fs *mockFS) ReadDirectory(path string) (map[string]*Entry, error) {
+	dir := fs.dirs[path]
+	if dir == nil {
+		return nil, syscall.ENOENT
+	}
+	return dir, nil
 }
 
 func (fs *mockFS) ReadFile(path string) (string, error) {
@@ -227,12 +231,17 @@ func (*mockFS) Rel(base string, target string) (string, bool) {
 
 type realFS struct {
 	// Stores the file entries for directories we've listed before
-	entries map[string]map[string]*Entry
+	entries map[string]entriesOrErr
 
 	// For the current working directory
 	cwd string
 }
 
+type entriesOrErr struct {
+	entries map[string]*Entry
+	err     error
+}
+
 // Limit the number of files open simultaneously to avoid ulimit issues
 var fileOpenLimit = make(chan bool, 32)
 
@@ -279,18 +288,18 @@ func RealFS() FS {
 		cwd = realpath(cwd)
 	}
 	return &realFS{
-		entries: make(map[string]map[string]*Entry),
+		entries: make(map[string]entriesOrErr),
 		cwd:     cwd,
 	}
 }
 
-func (fs *realFS) ReadDirectory(dir string) map[string]*Entry {
+func (fs *realFS) ReadDirectory(dir string) (map[string]*Entry, error) {
 	// First, check the cache
 	cached, ok := fs.entries[dir]
 
 	// Cache hit: stop now
 	if ok {
-		return cached
+		return cached.entries, cached.err
 	}
 
 	// Cache miss: read the directory entries
@@ -312,11 +321,13 @@ func (fs *realFS) ReadDirectory(dir string) map[string]*Entry {
 	// Update the cache unconditionally. Even if the read failed, we don't want to
 	// retry again later. The directory is inaccessible so trying again is wasted.
 	if err != nil {
-		fs.entries[dir] = nil
-		return nil
+		if pathErr, ok := err.(*os.PathError); ok {
+			err = pathErr.Unwrap()
+		}
+		entries = nil
 	}
-	fs.entries[dir] = entries
-	return entries
+	fs.entries[dir] = entriesOrErr{entries: entries, err: err}
+	return entries, err
 }
 
 func (fs *realFS) ReadFile(path string) (string, error) {
@@ -325,7 +336,7 @@ func (fs *realFS) ReadFile(path string) (string, error) {
 	buffer, err := ioutil.ReadFile(path)
 	if err != nil {
 		if pathErr, ok := err.(*os.PathError); ok {
-			return "", pathErr.Unwrap()
+			err = pathErr.Unwrap()
 		}
 	}
 	return string(buffer), err

internal/fs/fs_test.go

@@ -38,23 +38,23 @@ func TestBasic(t *testing.T) {
 	}
 
 	// Test a missing directory
-	missing := fs.ReadDirectory("/missing")
-	if missing != nil {
+	_, err = fs.ReadDirectory("/missing")
+	if err == nil {
 		t.Fatal("Unexpectedly found /missing")
 	}
 
 	// Test a nested directory
-	src := fs.ReadDirectory("/src")
-	if src == nil {
+	src, err := fs.ReadDirectory("/src")
+	if err != nil {
 		t.Fatal("Expected to find /src")
 	}
 	if len(src) != 2 || src["index.js"].Kind() != FileEntry || src["util.js"].Kind() != FileEntry {
 		t.Fatalf("Incorrect contents for /src: %v", src)
 	}
 
 	// Test the top-level directory
-	slash := fs.ReadDirectory("/")
-	if slash == nil {
+	slash, err := fs.ReadDirectory("/")
+	if err != nil {
 		t.Fatal("Expected to find /")
 	}
 	if len(slash) != 3 || slash["src"].Kind() != DirEntry || slash["README.md"].Kind() != FileEntry || slash["package.json"].Kind() != FileEntry {

internal/resolver/resolver.go

@@ -601,7 +601,14 @@ func (r *resolver) dirInfoUncached(path string) *dirInfo {
 	}
 
 	// List the directories
-	entries := r.fs.ReadDirectory(path)
+	entries, err := r.fs.ReadDirectory(path)
+	if err != nil {
+		if err != syscall.ENOENT {
+			r.log.AddError(nil, ast.Loc{},
+				fmt.Sprintf("Cannot read directory %q: %s", r.PrettyPath(path), err.Error()))
+		}
+		return nil
+	}
 	info := &dirInfo{
 		absPath: path,
 		parent:  parentInfo,
@@ -701,13 +708,16 @@ func (r *resolver) parsePackageJSON(path string) *packageJson {
 	// "main" property is supposed to be CommonJS, and ES6 helps us generate
 	// better code.
 	mainPath := ""
+	var mainRange ast.Range
 	if moduleJson, _, ok := getProperty(json, "module"); ok {
 		if main, ok := getString(moduleJson); ok {
 			mainPath = r.fs.Join(path, main)
+			mainRange = jsonSource.RangeOfString(moduleJson.Loc)
 		}
 	} else if mainJson, _, ok := getProperty(json, "main"); ok {
 		if main, ok := getString(mainJson); ok {
 			mainPath = r.fs.Join(path, main)
+			mainRange = jsonSource.RangeOfString(mainJson.Loc)
 		}
 	}
 
@@ -736,6 +746,7 @@ func (r *resolver) parsePackageJSON(path string) *packageJson {
 			//   },
 			//
 			mainPath = r.fs.Join(path, browser)
+			mainRange = jsonSource.RangeOfString(browserJson.Loc)
 		} else if browser, ok := browserJson.Data.(*ast.EObject); ok {
 			// The value is an object
 			browserPackageMap := make(map[string]*string)
@@ -811,11 +822,15 @@ func (r *resolver) parsePackageJSON(path string) *packageJson {
 			packageJson.absPathMain = &absolute
 		} else {
 			// Is it a directory?
-			if mainEntries := r.fs.ReadDirectory(mainPath); mainEntries != nil {
+			mainEntries, err := r.fs.ReadDirectory(mainPath)
+			if err == nil {
 				// Look for an "index" file with known extensions
 				if absolute, ok = r.loadAsIndex(mainPath, mainEntries); ok {
 					packageJson.absPathMain = &absolute
 				}
+			} else if err != syscall.ENOENT {
+				r.log.AddRangeError(&jsonSource, mainRange,
+					fmt.Sprintf("Cannot read directory %q: %s", r.PrettyPath(mainPath), err.Error()))
 			}
 		}
 	}
@@ -825,46 +840,52 @@ func (r *resolver) parsePackageJSON(path string) *packageJson {
 
 func (r *resolver) loadAsFile(path string) (string, bool) {
 	// Read the directory entries once to minimize locking
-	entries := r.fs.ReadDirectory(r.fs.Dir(path))
+	dirPath := r.fs.Dir(path)
+	entries, err := r.fs.ReadDirectory(dirPath)
+	if err != nil {
+		if err != syscall.ENOENT {
+			r.log.AddError(nil, ast.Loc{},
+				fmt.Sprintf("Cannot read directory %q: %s", r.PrettyPath(dirPath), err.Error()))
+		}
+		return "", false
+	}
 
-	if entries != nil {
-		base := r.fs.Base(path)
+	base := r.fs.Base(path)
 
-		// Try the plain path without any extensions
-		if entry, ok := entries[base]; ok && entry.Kind() == fs.FileEntry {
-			return path, true
-		}
+	// Try the plain path without any extensions
+	if entry, ok := entries[base]; ok && entry.Kind() == fs.FileEntry {
+		return path, true
+	}
 
-		// Try the path with extensions
-		for _, ext := range r.options.ExtensionOrder {
-			if entry, ok := entries[base+ext]; ok && entry.Kind() == fs.FileEntry {
-				return path + ext, true
-			}
+	// Try the path with extensions
+	for _, ext := range r.options.ExtensionOrder {
+		if entry, ok := entries[base+ext]; ok && entry.Kind() == fs.FileEntry {
+			return path + ext, true
 		}
+	}
 
-		// TypeScript-specific behavior: if the extension is ".js" or ".jsx", try
-		// replacing it with ".ts" or ".tsx". At the time of writing this specific
-		// behavior comes from the function "loadModuleFromFile()" in the file
-		// "moduleNameResolver.ts" in the TypeScript compiler source code. It
-		// contains this comment:
-		//
-		//   If that didn't work, try stripping a ".js" or ".jsx" extension and
-		//   replacing it with a TypeScript one; e.g. "./foo.js" can be matched
-		//   by "./foo.ts" or "./foo.d.ts"
-		//
-		// We don't care about ".d.ts" files because we can't do anything with
-		// those, so we ignore that part of the behavior.
-		//
-		// See the discussion here for more historical context:
-		// https://github.com/microsoft/TypeScript/issues/4595
-		if strings.HasSuffix(base, ".js") || strings.HasSuffix(base, ".jsx") {
-			lastDot := strings.LastIndexByte(base, '.')
-			// Note that the official compiler code always tries ".ts" before
-			// ".tsx" even if the original extension was ".jsx".
-			for _, ext := range []string{".ts", ".tsx"} {
-				if entry, ok := entries[base[:lastDot]+ext]; ok && entry.Kind() == fs.FileEntry {
-					return path[:len(path)-(len(base)-lastDot)] + ext, true
-				}
+	// TypeScript-specific behavior: if the extension is ".js" or ".jsx", try
+	// replacing it with ".ts" or ".tsx". At the time of writing this specific
+	// behavior comes from the function "loadModuleFromFile()" in the file
+	// "moduleNameResolver.ts" in the TypeScript compiler source code. It
+	// contains this comment:
+	//
+	//   If that didn't work, try stripping a ".js" or ".jsx" extension and
+	//   replacing it with a TypeScript one; e.g. "./foo.js" can be matched
+	//   by "./foo.ts" or "./foo.d.ts"
+	//
+	// We don't care about ".d.ts" files because we can't do anything with
+	// those, so we ignore that part of the behavior.
+	//
+	// See the discussion here for more historical context:
+	// https://github.com/microsoft/TypeScript/issues/4595
+	if strings.HasSuffix(base, ".js") || strings.HasSuffix(base, ".jsx") {
+		lastDot := strings.LastIndexByte(base, '.')
+		// Note that the official compiler code always tries ".ts" before
+		// ".tsx" even if the original extension was ".jsx".
+		for _, ext := range []string{".ts", ".tsx"} {
+			if entry, ok := entries[base[:lastDot]+ext]; ok && entry.Kind() == fs.FileEntry {
+				return path[:len(path)-(len(base)-lastDot)] + ext, true
 			}
 		}
 	}