fix(compiler)!: extern methods are not resolved correctly (#4043)

Previously, the compiler resolved strings in `extern` methods by performing a lookup, starting at the project's root directory. However, in multi-file projects this gives incorrect results since the source file might be inside a subdirectory of the user's project (or in the case of Wing libraries, inside their `node_modules` folder), resulting in bugs like #3916.

To fix this, we now resolve all paths into absolute paths while we're parsing all of the files, so that absolute paths are stored in most places in the compiler (including e.g. bring statement and extern statement AST nodes).

Fixes #3916
Fixes #4431

BREAKING CHANGE: `extern` statements can no longer refer to npm libraries, like `extern "uuid" pub static v4(): str;`. Instead, you can import the library in a JavaScript file and use the functionality through a regular export:

```js
// main.w
class Helpers {
  extern "./util.js" static v4(): str;
}

// util.js
exports.v4 = function() {
  return require("uuid").v4();
};
```

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [ ] Docs updated (only required for features)
- [x] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.

apps/wing-console/console/server/src/utils/format-wing-error.ts

@@ -43,8 +43,9 @@ export const formatWingError = async (error: unknown) => {
 
       for (const error of errors) {
         const { message, span, annotations } = error;
-        let files: File[] = [];
-        let labels: Label[] = [];
+        const files: File[] = [];
+        const labels: Label[] = [];
+        const cwd = process.cwd();
 
         // file_id might be "" if the span is synthetic (see #2521)
         if (span !== null && span !== undefined && span.file_id) {
@@ -60,9 +61,10 @@ export const formatWingError = async (error: unknown) => {
             span.end.line,
             span.end.col,
           );
-          files.push({ name: span.file_id, source });
+          const filePath = relative(cwd, span.file_id);
+          files.push({ name: filePath, source });
           labels.push({
-            fileId: span.file_id,
+            fileId: filePath,
             rangeStart: start,
             rangeEnd: end,
             message,
@@ -82,9 +84,10 @@ export const formatWingError = async (error: unknown) => {
             annotation.span.end.line,
             annotation.span.end.col,
           );
-          files.push({ name: annotation.span.file_id, source });
+          const filePath = relative(cwd, annotation.span.file_id);
+          files.push({ name: filePath, source });
           labels.push({
-            fileId: annotation.span.file_id,
+            fileId: filePath,
             rangeStart: start,
             rangeEnd: end,
             message: annotation.message,

apps/wing/src/commands/compile.ts

@@ -94,22 +94,24 @@ export async function compile(entrypoint?: string, options?: CompileOptions): Pr
     if (error instanceof wingCompiler.CompileError) {
       // This is a bug in the user's code. Print the compiler diagnostics.
       const diagnostics = error.diagnostics;
+      const cwd = process.cwd();
       const result = [];
 
       for (const diagnostic of diagnostics) {
         const { message, span, annotations } = diagnostic;
-        let files: File[] = [];
-        let labels: Label[] = [];
+        const files: File[] = [];
+        const labels: Label[] = [];
 
         // file_id might be "" if the span is synthetic (see #2521)
         if (span?.file_id) {
           // `span` should only be null if source file couldn't be read etc.
           const source = await fsPromise.readFile(span.file_id, "utf8");
           const start = byteOffsetFromLineAndColumn(source, span.start.line, span.start.col);
           const end = byteOffsetFromLineAndColumn(source, span.end.line, span.end.col);
-          files.push({ name: span.file_id, source });
+          const filePath = relative(cwd, span.file_id);
+          files.push({ name: filePath, source });
           labels.push({
-            fileId: span.file_id,
+            fileId: filePath,
             rangeStart: start,
             rangeEnd: end,
             message,
@@ -129,9 +131,10 @@ export async function compile(entrypoint?: string, options?: CompileOptions): Pr
             annotation.span.end.line,
             annotation.span.end.col
           );
-          files.push({ name: annotation.span.file_id, source });
+          const filePath = relative(cwd, annotation.span.file_id);
+          files.push({ name: filePath, source });
           labels.push({
-            fileId: annotation.span.file_id,
+            fileId: filePath,
             rangeStart: start,
             rangeEnd: end,
             message: annotation.message,

docs/docs/03-language-reference.md

@@ -1840,7 +1840,7 @@ let bucket = new awscdk.aws_s3.Bucket(
 
 ## 5.2 JavaScript
 
-The `extern "<commonjs module path or name>"` modifier can be used on method declarations in classes to indicate that a method is backed by an implementation imported from a JavaScript module. The module can either be a relative path or a name and will be loaded via [require()](https://nodejs.org/api/modules.html#requireid).
+The `extern "<commonjs module path>"` modifier can be used on method declarations in classes to indicate that a method is backed by an implementation imported from a JavaScript module. The module must be a relative path and will be loaded via [require()](https://nodejs.org/api/modules.html#requireid).
 
 In the following example, the static inflight method `makeId` is implemented
 in `helper.js`:
@@ -1860,9 +1860,6 @@ class TaskList {
 
   // Load js helper file
   extern "./helpers.js" static inflight makeId(): str;
-
-  // Alternatively, you can use a module name
-  extern "uuid" static inflight v4(): str;
 } 
 
 // helpers.js

examples/tests/invalid/extern.test.w

@@ -1,5 +1,4 @@
 class Foo {
   extern "./sad.js" static getNum(): num;
 //^ "./sad.js" not found
-  extern "not-installed" static tooBad(): bool;
 }

examples/tests/invalid/extern_static.test.w

@@ -1,4 +1,4 @@
 class Foo {
-  extern "../external_js.js" inflight getGreeting(name: str): str;
+  extern "../valid/external_js.js" inflight getGreeting(name: str): str;
                                     //^ Error: extern methods must be declared "static"
 }

examples/tests/valid/bring_wing_library.test.w

@@ -2,3 +2,9 @@ bring "wing-fixture" as fixture;
 
 new fixture.Store();
 let fave_num = fixture.FavoriteNumbers.SEVEN;
+
+assert(fixture.Store.makeKey("hello") == "data/hello.json");
+
+test "makeKeyInflight" {
+  assert(fixture.Store.makeKeyInflight("hello") == "data/hello.json");
+}

examples/tests/valid/extern_implementation.test.w

@@ -6,7 +6,6 @@ class Foo {
   extern "./external_js.js" static inflight getUuid(): str;
   extern "./external_js.js" static inflight getData(): str;
   extern "./external_js.js" pub static inflight print(msg: str);
-  extern "uuid" pub static v4(): str;
 
   pub inflight call() {
     assert(Foo.regexInflight("[a-z]+-\\d+", "abc-123"));
@@ -18,7 +17,6 @@ class Foo {
 }
 
 assert(Foo.getGreeting("Wingding") == "Hello, Wingding!");
-assert(Foo.v4().length == 36);
 
 let f = new Foo();
 

examples/tests/valid/subdir/subfile.w

@@ -1,3 +1,5 @@
 bring math;
 
-class Q {}
+class Q {
+  extern "./util.js" static inflight greet(name: str): str;
+}

examples/tests/valid/subdir/util.js

@@ -0,0 +1,3 @@
+exports.greet = function(name) {
+    return 'Hello ' + name;
+}

examples/wing-fixture/store.w

@@ -7,6 +7,9 @@ class Store {
     this.data = new cloud.Bucket();
   }
 
+  extern "./util.js" pub static makeKey(name: str): str;
+  extern "./util.js" pub static inflight makeKeyInflight(name: str): str;
+
   inflight set(message: str) {
     this.data.put("data.txt", myutil.double(message));
   }

examples/wing-fixture/util.js

@@ -0,0 +1,5 @@
+exports.makeKey = function(name) {
+    return "data/" + name + ".json";
+}
+
+exports.makeKeyInflight = exports.makeKey;

libs/wingc/examples/compile.rs

@@ -15,7 +15,7 @@ pub fn main() {
 	let source_path = Utf8Path::new(&args[1]).canonicalize_utf8().unwrap();
 	let target_dir: Utf8PathBuf = env::current_dir().unwrap().join("target").try_into().unwrap();
 
-	let results = compile(&source_path, None, &target_dir, source_path.parent().unwrap());
+	let results = compile(source_path.parent().unwrap(), &source_path, None, &target_dir);
 	if results.is_err() {
 		let mut diags = get_diagnostics();
 		// Sort error messages by line number (ascending)

libs/wingc/src/diagnostic.rs

@@ -1,3 +1,4 @@
+use camino::Utf8Path;
 use colored::Colorize;
 use std::{cell::RefCell, fmt::Display};
 use tree_sitter::Point;
@@ -205,9 +206,15 @@ impl WingSpan {
 	}
 }
 
-impl std::fmt::Display for WingSpan {
+impl Display for WingSpan {
 	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-		write!(f, "{}:{}:{}", self.file_id, self.start.line + 1, self.start.col + 1)
+		write!(
+			f,
+			"{}:{}:{}",
+			Utf8Path::new(&self.file_id).file_name().expect("invalid file id"),
+			self.start.line + 1,
+			self.start.col + 1
+		)
 	}
 }
 

libs/wingc/src/jsify.rs

@@ -15,7 +15,7 @@ use crate::{
 		StmtKind, Symbol, UnaryOperator, UserDefinedType,
 	},
 	comp_ctx::{CompilationContext, CompilationPhase},
-	dbg_panic, debug,
+	dbg_panic,
 	diagnostic::{report_diagnostic, Diagnostic, WingSpan},
 	file_graph::FileGraph,
 	files::Files,
@@ -71,8 +71,6 @@ pub struct JSifier<'a> {
 	preflight_file_map: RefCell<IndexMap<Utf8PathBuf, String>>,
 	source_files: &'a Files,
 	source_file_graph: &'a FileGraph,
-	/// Root of the project, used for resolving extern modules
-	absolute_project_root: &'a Utf8Path,
 	/// The path that compilation started at (file or directory)
 	compilation_init_path: &'a Utf8Path,
 }
@@ -91,15 +89,13 @@ impl<'a> JSifier<'a> {
 		source_files: &'a Files,
 		source_file_graph: &'a FileGraph,
 		compilation_init_path: &'a Utf8Path,
-		absolute_project_root: &'a Utf8Path,
 	) -> Self {
 		let output_files = Files::default();
 		Self {
 			types,
 			source_files,
 			source_file_graph,
 			compilation_init_path,
-			absolute_project_root,
 			referenced_struct_schemas: RefCell::new(BTreeMap::new()),
 			inflight_file_counter: RefCell::new(0),
 			inflight_file_map: RefCell::new(IndexMap::new()),
@@ -1083,26 +1079,8 @@ impl<'a> JSifier<'a> {
 				code.add_code(self.jsify_scope_body(scope, ctx));
 				code
 			}
-			FunctionBody::External(external_spec) => {
-				debug!(
-					"Resolving extern \"{}\" from \"{}\"",
-					external_spec, self.absolute_project_root
-				);
-				let resolved_path =
-					match wingii::node_resolve::resolve_from(&external_spec, Utf8Path::new(&self.absolute_project_root)) {
-						Ok(resolved_path) => resolved_path.to_string().replace("\\", "/"),
-						Err(err) => {
-							report_diagnostic(Diagnostic {
-								message: format!("Failed to resolve extern \"{external_spec}\": {err}"),
-								span: Some(func_def.span.clone()),
-								annotations: vec![],
-							});
-							format!("/* unresolved: \"{external_spec}\" */")
-						}
-					};
-				CodeMaker::one_line(format!(
-					"return (require(\"{resolved_path}\")[\"{name}\"])({parameters})"
-				))
+			FunctionBody::External(file_path) => {
+				CodeMaker::one_line(format!("return (require(\"{file_path}\")[\"{name}\"])({parameters})"))
 			}
 		};
 		let mut prefix = vec![];

libs/wingc/src/lib.rs

@@ -171,8 +171,11 @@ pub unsafe extern "C" fn wingc_compile(ptr: u32, len: u32) -> u64 {
 		return WASM_RETURN_ERROR;
 	}
 	let source_path = Utf8Path::new(split[0]);
-	let output_dir = split.get(1).map(|s| Utf8Path::new(s)).unwrap();
-	let absolute_project_dir = split.get(2).map(|s| Utf8Path::new(s)).unwrap();
+	let output_dir = split.get(1).map(|s| Utf8Path::new(s)).expect("output dir not provided");
+	let project_dir = split
+		.get(2)
+		.map(|s| Utf8Path::new(s))
+		.expect("project dir not provided");
 
 	if !source_path.exists() {
 		report_diagnostic(Diagnostic {
@@ -183,7 +186,7 @@ pub unsafe extern "C" fn wingc_compile(ptr: u32, len: u32) -> u64 {
 		return WASM_RETURN_ERROR;
 	}
 
-	let results = compile(source_path, None, output_dir, absolute_project_dir);
+	let results = compile(project_dir, source_path, None, output_dir);
 	if results.is_err() {
 		WASM_RETURN_ERROR
 	} else {
@@ -287,10 +290,10 @@ fn add_builtin(name: &str, typ: Type, scope: &mut Scope, types: &mut Types) {
 }
 
 pub fn compile(
+	project_dir: &Utf8Path,
 	source_path: &Utf8Path,
 	source_text: Option<String>,
 	out_dir: &Utf8Path,
-	absolute_project_root: &Utf8Path,
 ) -> Result<CompilerOutput, ()> {
 	let source_path = normalize_path(source_path, None);
 
@@ -357,8 +360,6 @@ pub fn compile(
 		asts.insert(file.clone(), scope);
 	}
 
-	let project_dir = absolute_project_root;
-
 	// Verify that the project dir is absolute
 	if !is_absolute_path(&project_dir) {
 		report_diagnostic(Diagnostic {
@@ -369,7 +370,7 @@ pub fn compile(
 		return Err(());
 	}
 
-	let mut jsifier = JSifier::new(&mut types, &files, &file_graph, &source_path, &project_dir);
+	let mut jsifier = JSifier::new(&mut types, &files, &file_graph, &source_path);
 
 	// -- LIFTING PHASE --
 
@@ -435,39 +436,34 @@ pub fn is_absolute_path(path: &Utf8Path) -> bool {
 
 #[cfg(test)]
 mod sanity {
-	use camino::Utf8PathBuf;
+	use camino::{Utf8Path, Utf8PathBuf};
 
 	use crate::{compile, diagnostic::assert_no_panics};
-	use std::{fs, path::Path};
+	use std::fs;
 
 	fn get_wing_files<P>(dir: P) -> impl Iterator<Item = Utf8PathBuf>
 	where
-		P: AsRef<Path>,
+		P: AsRef<Utf8Path>,
 	{
-		fs::read_dir(dir)
+		fs::read_dir(dir.as_ref())
 			.unwrap()
 			.map(|entry| Utf8PathBuf::from_path_buf(entry.unwrap().path()).expect("invalid unicode path"))
 			.filter(|path| path.is_file() && path.extension().map(|ext| ext == "w").unwrap_or(false))
 	}
 
 	fn compile_test(test_dir: &str, expect_failure: bool) {
-		for test_file in get_wing_files(test_dir) {
+		let test_dir = Utf8Path::new(test_dir).canonicalize_utf8().unwrap();
+		for test_file in get_wing_files(&test_dir) {
 			println!("\n=== {} ===\n", test_file);
 
-			let mut out_dir = test_file.parent().unwrap().to_path_buf();
-			out_dir.push(format!("target/wingc/{}.out", test_file.file_name().unwrap()));
+			let out_dir = test_dir.join(format!("target/wingc/{}.out", test_file.file_name().unwrap()));
 
 			// reset out_dir
 			if out_dir.exists() {
 				fs::remove_dir_all(&out_dir).expect("remove out dir");
 			}
 
-			let result = compile(
-				&test_file,
-				None,
-				&out_dir,
-				test_file.canonicalize_utf8().unwrap().parent().unwrap(),
-			);
+			let result = compile(&test_dir, &test_file, None, &out_dir);
 
 			if result.is_err() {
 				assert!(

libs/wingc/src/lsp/sync.rs

@@ -195,16 +195,7 @@ fn partial_compile(
 
 	// -- LIFTING PHASE --
 
-	// source_file will never be "" because it is the path to the file being compiled and lsp does not allow empty paths
-	let project_dir = source_path.parent().expect("Empty filename");
-
-	let jsifier = JSifier::new(
-		&mut types,
-		&project_data.files,
-		&project_data.file_graph,
-		&source_path,
-		&project_dir,
-	);
+	let jsifier = JSifier::new(&mut types, &project_data.files, &project_data.file_graph, &source_path);
 	for file in &topo_sorted_files {
 		let mut lift = LiftVisitor::new(&jsifier);
 		let scope = project_data.asts.remove(file).expect("matching AST not found");

libs/wingc/src/parser.rs

@@ -1393,8 +1393,12 @@ impl<'s> Parser<'s> {
 		let signature = self.build_function_signature(func_def_node, phase)?;
 		let statements = if let Some(external) = func_def_node.child_by_field_name("extern_modifier") {
 			let node_text = self.node_text(&external.named_child(0).unwrap());
-			let node_text = &node_text[1..node_text.len() - 1];
-			FunctionBody::External(node_text.to_string())
+			let file_path = Utf8Path::new(&node_text[1..node_text.len() - 1]);
+			let file_path = normalize_path(file_path, Some(&Utf8Path::new(&self.source_name)));
+			if !file_path.exists() {
+				self.add_error(format!("File not found: {}", node_text), &external);
+			}
+			FunctionBody::External(file_path.to_string())
 		} else {
 			FunctionBody::Statements(self.build_scope(&self.get_child_field(func_def_node, "block")?, phase))
 		};