add pnp error handling and test (#3)

* add pnpapi test

* remove cycle dependency

* add error handling

* add error message

* apply error message

* add error bubbling

* code clean up

* remove useless test

* add empty findBrokenPeerDependencies

* early return

* change function name

* apply code review

* merge duplicate logic

* apply collect usage

* apply codereview

* apply fromConfig

Author: gun-yu

internal/core/compileroptions.go

@@ -6,7 +6,6 @@ import (
 	"sync"
 
 	"github.com/microsoft/typescript-go/internal/collections"
-	"github.com/microsoft/typescript-go/internal/pnp"
 	"github.com/microsoft/typescript-go/internal/tspath"
 )
 
@@ -301,10 +300,17 @@ func (options *CompilerOptions) GetStrictOptionValue(value Tristate) bool {
 	return options.Strict == TSTrue
 }
 
-func (options *CompilerOptions) GetEffectiveTypeRoots(currentDirectory string, pnpApi *pnp.PnpApi) (result []string, fromConfig bool) {
+func (options *CompilerOptions) GetEffectiveTypeRoots(currentDirectory string) (result []string, fromConfig bool) {
 	if options.TypeRoots != nil {
 		return options.TypeRoots, true
 	}
+	baseDir := options.GetBaseDirFromOptions(currentDirectory)
+
+	nmTypes, nmFromConfig := options.GetNodeModulesTypeRoots(baseDir)
+	return nmTypes, nmFromConfig
+}
+
+func (options *CompilerOptions) GetBaseDirFromOptions(currentDirectory string) string {
 	var baseDir string
 	if options.ConfigFilePath != "" {
 		baseDir = tspath.GetDirectoryPath(options.ConfigFilePath)
@@ -316,15 +322,7 @@ func (options *CompilerOptions) GetEffectiveTypeRoots(currentDirectory string, p
 			panic("cannot get effective type roots without a config file path or current directory")
 		}
 	}
-
-	nmTypes, nmFromConfig := options.GetNodeModulesTypeRoots(baseDir)
-
-	if pnpApi != nil {
-		typeRoots, fromConfig := pnpApi.AppendPnpTypeRoots(nmTypes, baseDir, nmFromConfig)
-		return typeRoots, fromConfig
-	}
-
-	return nmTypes, nmFromConfig
+	return baseDir
 }
 
 func (options *CompilerOptions) GetNodeModulesTypeRoots(baseDir string) (result []string, fromConfig bool) {

internal/diagnostics/diagnostics_generated.go

@@ -34,5 +34,41 @@
     "Project '{0}' is out of date because it has errors.": {
         "category": "Message",
         "code": 6423
+    },
+    "Your application tried to access '{0}'. While this module is usually interpreted as a Node builtin, your resolver is running inside a non-Node resolution context where such builtins are ignored. Since '{0}' isn't otherwise declared in your dependencies, this makes the require call ambiguous and unsound.\n\nRequired package: {0}{1}\nRequired by: {2}": {
+        "category": "Error",
+        "code": 100003
+    },
+    "{0} tried to access '{1}'. While this module is usually interpreted as a Node builtin, your resolver is running inside a non-Node resolution context where such builtins are ignored. Since '{1}' isn't otherwise declared in {0}'s dependencies, this makes the require call ambiguous and unsound.\n\nRequired package: {1}{2}\nRequired by: {3}": {
+        "category": "Error",
+        "code": 100004
+    },
+    "Your application tried to access '{0}', but it isn't declared in your dependencies; this makes the require call ambiguous and unsound.\n\nRequired package: {0}{1}\nRequired by: {2}": {
+        "category": "Error",
+        "code": 100005
+    },
+    "Your application tried to access '{0}' (a peer dependency); this isn't allowed as there is no ancestor to satisfy the requirement. Use a devDependency if needed.\n\nRequired package: {0}\nRequired by: {1}": {
+        "category": "Error",
+        "code": 100006
+    },
+    "{0} tried to access '{1}' (a peer dependency) but it isn't provided by its ancestors/your application; this makes the require call ambiguous and unsound.\n\nRequired package: {1}\nRequired by: {2}": {
+        "category": "Error",
+        "code": 100007
+    },
+    "no PnP manifest found": {
+        "category": "Error",
+        "code": 100008
+    },
+    "no package found for path '{0}'": {
+        "category": "Error",
+        "code": 100009
+    },
+    "Empty specifier: '{0}'": {
+        "category": "Error",
+        "code": 100010
+    },
+    "Invalid specifier: '{0}'": {
+        "category": "Error",
+        "code": 100011
     }
 }

internal/diagnostics/extraDiagnosticMessages.json

@@ -207,7 +207,10 @@ func (r *Resolver) ResolveTypeReferenceDirective(
 	compilerOptions := GetCompilerOptionsWithRedirect(r.compilerOptions, redirectedReference)
 	containingDirectory := tspath.GetDirectoryPath(containingFile)
 
-	typeRoots, fromConfig := compilerOptions.GetEffectiveTypeRoots(r.host.GetCurrentDirectory(), r.host.PnpApi())
+	typeRoots, fromConfig := compilerOptions.GetEffectiveTypeRoots(r.host.GetCurrentDirectory())
+	if pnpApi := r.host.PnpApi(); pnpApi != nil {
+		typeRoots, fromConfig = pnpApi.AppendPnpTypeRoots(typeRoots, r.host.GetCurrentDirectory(), compilerOptions, fromConfig)
+	}
 	if traceBuilder != nil {
 		traceBuilder.write(diagnostics.Resolving_type_reference_directive_0_containing_file_1_root_directory_2.Format(typeReferenceDirectiveName, containingFile, strings.Join(typeRoots, ",")))
 		traceBuilder.traceResolutionUsingProjectReference(redirectedReference)
@@ -983,8 +986,13 @@ func (r *resolutionState) loadModuleFromPnpResolution(ext extensions, moduleName
 
 	if pnpApi != nil {
 		packageName, rest := ParsePackageName(moduleName)
-		// TODO: bubble up yarn resolution errors, instead of _
-		packageDirectory, _ := pnpApi.ResolveToUnqualified(packageName, issuer)
+		packageDirectory, err := pnpApi.ResolveToUnqualified(packageName, issuer)
+		if err != nil {
+			if r.tracer != nil {
+				r.tracer.write(err.Error())
+			}
+			return nil
+		}
 		if packageDirectory != "" {
 			candidate := tspath.NormalizePath(tspath.CombinePaths(packageDirectory, rest))
 			return r.loadModuleFromSpecificNodeModulesDirectoryImpl(ext, true /* nodeModulesDirectoryExists */, candidate, rest, packageDirectory)
@@ -1790,7 +1798,14 @@ func (r *resolutionState) readPackageJsonPeerDependencies(packageJsonInfo *packa
 		var peerDependencyPath string
 
 		if pnpApi != nil {
-			peerDependencyPath, _ = pnpApi.ResolveToUnqualified(name, packageDirectory)
+			var err error
+			peerDependencyPath, err = pnpApi.ResolveToUnqualified(name, packageDirectory)
+			if err != nil {
+				if r.tracer != nil {
+					r.tracer.write(err.Error())
+				}
+				continue
+			}
 		}
 
 		if peerDependencyPath == "" {
@@ -2040,7 +2055,10 @@ func GetAutomaticTypeDirectiveNames(options *core.CompilerOptions, host Resoluti
 	}
 
 	var result []string
-	typeRoots, _ := options.GetEffectiveTypeRoots(host.GetCurrentDirectory(), host.PnpApi())
+	typeRoots, fromConfig := options.GetEffectiveTypeRoots(host.GetCurrentDirectory())
+	if pnpApi := host.PnpApi(); pnpApi != nil {
+		typeRoots, fromConfig = pnpApi.AppendPnpTypeRoots(typeRoots, host.GetCurrentDirectory(), options, fromConfig)
+	}
 	for _, root := range typeRoots {
 		if host.FS().DirectoryExists(root) {
 			for _, typeDirectivePath := range host.FS().GetAccessibleEntries(root).Directories {

internal/module/resolver.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/dlclark/regexp2"
 	"github.com/microsoft/typescript-go/internal/tspath"
+	"github.com/microsoft/typescript-go/internal/vfs"
 )
 
 type LinkType string
@@ -74,40 +75,47 @@ type PnpManifestData struct {
 	packageRegistryTrie *PackageRegistryTrie
 }
 
-func parseManifestFromPath(fs PnpApiFS, manifestDir string) (*PnpManifestData, error) {
+func parseManifestFromPath(fs vfs.FS, manifestDir string) (*PnpManifestData, error) {
 	pnpDataString := ""
 
 	data, ok := fs.ReadFile(tspath.CombinePaths(manifestDir, ".pnp.data.json"))
 	if ok {
 		pnpDataString = data
 	} else {
-		pnpScriptString, ok := fs.ReadFile(tspath.CombinePaths(manifestDir, ".pnp.cjs"))
-		if !ok {
-			return nil, errors.New("failed to read .pnp.cjs file")
+		dataString, err := extractPnpDataStringFromPath(fs, tspath.CombinePaths(manifestDir, ".pnp.cjs"))
+		if err != nil {
+			return nil, err
 		}
+		pnpDataString = dataString
+	}
 
-		manifestRegex := regexp2.MustCompile(`(const[ \r\n]+RAW_RUNTIME_STATE[ \r\n]*=[ \r\n]*|hydrateRuntimeState\(JSON\.parse\()'`, regexp2.None)
-		matches, err := manifestRegex.FindStringMatch(pnpScriptString)
-		if err != nil || matches == nil {
-			return nil, errors.New("We failed to locate the PnP data payload inside its manifest file. Did you manually edit the file?")
-		}
+	return parseManifestFromData(pnpDataString, manifestDir)
+}
 
-		start := matches.Index + matches.Length
-		var b strings.Builder
-		b.Grow(len(pnpScriptString))
-		for i := start; i < len(pnpScriptString); i++ {
-			if pnpScriptString[i] == '\'' {
-				break
-			}
+func extractPnpDataStringFromPath(fs vfs.FS, path string) (string, error) {
+	pnpScriptString, ok := fs.ReadFile(path)
+	if !ok {
+		return "", errors.New("failed to read file: " + path)
+	}
+	manifestRegex := regexp2.MustCompile(`(const[ \r\n]+RAW_RUNTIME_STATE[ \r\n]*=[ \r\n]*|hydrateRuntimeState\(JSON\.parse\()'`, regexp2.None)
+	matches, err := manifestRegex.FindStringMatch(pnpScriptString)
+	if err != nil || matches == nil {
+		return "", errors.New("we failed to locate the PnP data payload inside its manifest file. Did you manually edit the file?")
+	}
 
-			if pnpScriptString[i] != '\\' {
-				b.WriteByte(pnpScriptString[i])
-			}
+	start := matches.Index + matches.Length
+	var b strings.Builder
+	b.Grow(len(pnpScriptString))
+	for i := start; i < len(pnpScriptString); i++ {
+		if pnpScriptString[i] == '\'' {
+			break
 		}
-		pnpDataString = b.String()
-	}
 
-	return parseManifestFromData(pnpDataString, manifestDir)
+		if pnpScriptString[i] != '\\' {
+			b.WriteByte(pnpScriptString[i])
+		}
+	}
+	return b.String(), nil
 }
 
 func parseManifestFromData(pnpDataString string, manifestDir string) (*PnpManifestData, error) {

internal/pnp/manifestparser.go

@@ -1,8 +1,12 @@
 package pnp
 
-import "strings"
+import (
+	"strings"
 
-func InitPnpApi(fs PnpApiFS, filePath string) *PnpApi {
+	"github.com/microsoft/typescript-go/internal/vfs"
+)
+
+func InitPnpApi(fs vfs.FS, filePath string) *PnpApi {
 	pnpApi := &PnpApi{fs: fs, url: filePath}
 
 	manifestData, err := pnpApi.findClosestPnpManifest()