From ca6d5bea6c7cf5d16ca65f9ee223d2d3457ab2ec Mon Sep 17 00:00:00 2001 From: Chris Rybicki Date: Thu, 31 Aug 2023 17:26:32 -0400 Subject: [PATCH] fix(compiler)!: non-static extern methods are not supported (#4027) It's not currently possible for extern method implementations to access class members (like fields, or other instance methods), yet it has still been possible to declare them both ways. This PR introduces an error message if you attempt to add a non-static extern method. In the future, this restriction can be lifted if the capability is supported through a new calling convention with extern JavaScript files or something like that. ## 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) - [ ] 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)*. --- docs/docs/03-language-reference.md | 2 ++ examples/tests/invalid/extern.w | 3 +-- examples/tests/invalid/extern_static.w | 4 ++++ examples/tests/valid/dynamo.w | 4 ++-- examples/tests/valid/dynamo_awscdk.w | 8 ++++---- examples/tests/valid/extern_implementation.w | 4 ++-- libs/wingc/src/type_check.rs | 9 +++++++++ tools/hangar/__snapshots__/invalid.ts.snap | 15 +++++++++++++++ .../extern_implementation.w_compile_tf-aws.md | 10 +++++----- 9 files changed, 44 insertions(+), 15 deletions(-) create mode 100644 examples/tests/invalid/extern_static.w diff --git a/docs/docs/03-language-reference.md b/docs/docs/03-language-reference.md index 95a1b77c1b0..8b33020cb30 100644 --- a/docs/docs/03-language-reference.md +++ b/docs/docs/03-language-reference.md @@ -1762,6 +1762,8 @@ exports.makeId = function () { Given a method of name X, the compiler will map the method to the JavaScript export with the matching name (without any case conversion). +Extern methods do not support access to class's members through `this`, so they must be declared `static`. + ### 5.2.1 TypeScript It is possible to use TypeScript to write helpers, but at the moment this is not diff --git a/examples/tests/invalid/extern.w b/examples/tests/invalid/extern.w index 3f8e333e8ef..5094dd32f60 100644 --- a/examples/tests/invalid/extern.w +++ b/examples/tests/invalid/extern.w @@ -2,5 +2,4 @@ class Foo { extern "./sad.js" static getNum(): num; //^ "./sad.js" not found extern "not-installed" static tooBad(): bool; -//^ "not-installed" not found -} \ No newline at end of file +} diff --git a/examples/tests/invalid/extern_static.w b/examples/tests/invalid/extern_static.w new file mode 100644 index 00000000000..8c59090ea22 --- /dev/null +++ b/examples/tests/invalid/extern_static.w @@ -0,0 +1,4 @@ +class Foo { + extern "../external_js.js" inflight getGreeting(name: str): str; + //^ Error: extern methods must be declared "static" +} diff --git a/examples/tests/valid/dynamo.w b/examples/tests/valid/dynamo.w index be442dca6e9..e235cf44c06 100644 --- a/examples/tests/valid/dynamo.w +++ b/examples/tests/valid/dynamo.w @@ -54,11 +54,11 @@ class DynamoTable { } } - extern "./dynamo.js" inflight _putItem(tableName: str, item: Json): void; + extern "./dynamo.js" static inflight _putItem(tableName: str, item: Json): void; inflight putItem(item: Map) { let json = this._itemToJson(item); - this._putItem(this.tableName, json); + DynamoTable._putItem(this.tableName, json); } inflight _itemToJson(item: Map): Json { diff --git a/examples/tests/valid/dynamo_awscdk.w b/examples/tests/valid/dynamo_awscdk.w index af609b94a7a..8c5f2c92959 100644 --- a/examples/tests/valid/dynamo_awscdk.w +++ b/examples/tests/valid/dynamo_awscdk.w @@ -60,16 +60,16 @@ class DynamoTable { } } - extern "./dynamo.js" inflight _putItem(tableName: str, item: Json): void; + extern "./dynamo.js" static inflight _putItem(tableName: str, item: Json): void; inflight putItem(item: Map) { let json = this._itemToJson(item); - this._putItem(this.tableName, json); + DynamoTable._putItem(this.tableName, json); } - extern "./dynamo.js" inflight _getItem(tableName: str, key: Json): Json; + extern "./dynamo.js" static inflight _getItem(tableName: str, key: Json): Json; inflight getItem(key: Map): Json { let json = this._itemToJson(key); - return this._getItem(this.tableName, json); + return DynamoTable._getItem(this.tableName, json); } inflight _itemToJson(item: Map): Json { diff --git a/examples/tests/valid/extern_implementation.w b/examples/tests/valid/extern_implementation.w index ca6e42f5354..b5fac4684dd 100644 --- a/examples/tests/valid/extern_implementation.w +++ b/examples/tests/valid/extern_implementation.w @@ -5,7 +5,7 @@ class Foo { extern "./external_js.js" static inflight regexInflight(pattern: str, text: str): bool; extern "./external_js.js" static inflight getUuid(): str; extern "./external_js.js" static inflight getData(): str; - extern "./external_js.js" inflight print(msg: str); + extern "./external_js.js" static inflight print(msg: str); extern "uuid" static v4(): str; inflight call() { @@ -27,5 +27,5 @@ test "call" { } test "console" { - f.print("hey there"); + Foo.print("hey there"); } diff --git a/libs/wingc/src/type_check.rs b/libs/wingc/src/type_check.rs index e5a31414d8d..e3052f3f921 100644 --- a/libs/wingc/src/type_check.rs +++ b/libs/wingc/src/type_check.rs @@ -3941,6 +3941,15 @@ impl<'a> TypeChecker<'a> { self.types.set_scope_env(scope, method_env); self.inner_scopes.push(scope); } + + if let FunctionBody::External(_) = &method_def.body { + if !method_def.is_static { + self.spanned_error( + method_name, + "Extern methods must be declared \"static\" (they cannot access instance members)", + ); + } + } } fn add_method_to_class_env( diff --git a/tools/hangar/__snapshots__/invalid.ts.snap b/tools/hangar/__snapshots__/invalid.ts.snap index e8b14a2625d..35674fef90b 100644 --- a/tools/hangar/__snapshots__/invalid.ts.snap +++ b/tools/hangar/__snapshots__/invalid.ts.snap @@ -752,6 +752,21 @@ error: Failed to resolve extern \\"not-installed\\": Not Found +Tests 1 failed (1) +Test Files 1 failed (1) +Duration " +`; + +exports[`extern_static.w 1`] = ` +"error: Extern methods must be declared \\"static\\" (they cannot access instance members) + --> ../../../examples/tests/invalid/extern_static.w:2:39 + | +2 | extern \\"../external_js.js\\" inflight getGreeting(name: str): str; + | ^^^^^^^^^^^ Extern methods must be declared \\"static\\" (they cannot access instance members) + + + + Tests 1 failed (1) Test Files 1 failed (1) Duration " diff --git a/tools/hangar/__snapshots__/test_corpus/valid/extern_implementation.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/extern_implementation.w_compile_tf-aws.md index e3f59932f67..b536e130438 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/extern_implementation.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/extern_implementation.w_compile_tf-aws.md @@ -20,7 +20,7 @@ module.exports = function({ $f }) { ## inflight.$Closure2-1.js ```js -module.exports = function({ $f }) { +module.exports = function({ $Foo }) { class $Closure2 { constructor({ }) { const $obj = (...args) => this.handle(...args); @@ -28,7 +28,7 @@ module.exports = function({ $f }) { return $obj; } async handle() { - (await $f.print("hey there")); + (await $Foo.print("hey there")); } } return $Closure2; @@ -51,7 +51,7 @@ module.exports = function({ }) { static async getData() { return (require("/external_js.js")["getData"])() } - async print(msg) { + static async print(msg) { return (require("/external_js.js")["print"])(msg) } async call() { @@ -348,7 +348,7 @@ class $Root extends $stdlib.std.Resource { static _toInflightType(context) { return ` require("./inflight.$Closure2-1.js")({ - $f: ${context._lift(f)}, + $Foo: ${context._lift(Foo)}, }) `; } @@ -368,7 +368,7 @@ class $Root extends $stdlib.std.Resource { } _registerBind(host, ops) { if (ops.includes("handle")) { - $Closure2._registerBindObject(f, host, ["print"]); + $Closure2._registerBindObject(Foo, host, ["print"]); } super._registerBind(host, ops); }