Skip to content

Commit

Permalink
feat(compiler)!: interfaces, enums, and public classes must be define…
Browse files Browse the repository at this point in the history
…d at the top-level (#7036)

Closes #7035
Closes #5542

To encourage more readable code and reduce the number of edge cases we might have to handle in the compiler implementation, we add the constraint that that interfaces, enums, and any exported classes have to be defined at the top level. This requirement is already in place for structs.

It's a bit unclear whether we want to apply the same restriction to classes, since classes can have code inside of them (unlike interfaces, enums, and structs) and you can certainly find plenty of examples where folks define anonymous classes inside functions in other languages, so for now we're continue to allow them, though support for it isn't as rigorously tested. We only require that if you define a class in a local context, it must be private, since it wouldn't be possible to access them in other Wing files.

BREAKING CHANGE: interfaces, enums, and classes marked with "pub" or "internal" are now required to be defined at the top-level of Wing programs. Please let us know if you encounter issues due to this change.

## 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)*.
  • Loading branch information
Chriscbr authored Aug 21, 2024
1 parent 39e5773 commit eca0508
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 61 deletions.
9 changes: 8 additions & 1 deletion docs/api/05-language-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -1365,6 +1365,7 @@ If no catch block exists among caller functions, the program will terminate.
Structs are loosely modeled after typed JSON literals in JavaScript.
Structs are defined with the `struct` keyword.
Structs are "bags" of immutable data.
Structs must be defined at the top-level of a Wing file.
Structs can only have fields of primitive types, preflight classes, and other structs.
Array, set, and map of above types is also allowed in struct field definition.
Expand Down Expand Up @@ -1529,6 +1530,9 @@ Multiple inheritance is invalid and forbidden.
Multiple implementations of various interfaces is allowed.
Multiple implementations of the same interface is invalid and forbidden.
Classes can have an [access modifier](#15-access-modifiers-member-visibility) specifying whether it can be imported by other Wing source files.
Classes can only be marked `pub` or `internal` if they are defined at the top-level of a Wing file.
In methods if return type is missing, `: void` is assumed.
#### Roadmap
Expand Down Expand Up @@ -1644,6 +1648,8 @@ of methods with different phases is not allowed as well.
Interfaces represent a contract that a class must fulfill.
Interfaces are defined with the `interface` keyword.
Interfaces may be either preflight interfaces or inflight interfaces.
Interfaces must be defined at the top-level of a Wing file.
Preflight interfaces are defined in preflight scope and can contain both preflight and inflight methods.
Only preflight classes may implement preflight interfaces.
Inflight interfaces are either defined with the `inflight` modifier in preflight scope or simply defined in inflight scope.
Expand Down Expand Up @@ -1809,7 +1815,8 @@ Arrays are similar to dynamically sized arrays or vectors in other languages.
Enumeration type (`enum`) is a type that groups a list of named constant members.
Enumeration is defined by writing **enum**, followed by enumeration name and a
list of comma-separated constants in a {}.
list of comma-separated constants in a {}.
Enums must be defined at the top-level of a Wing file.
Naming convention for enums is to use "TitleCase" for name and ALL_CAPS for members.
> ```TS
Expand Down
10 changes: 10 additions & 0 deletions examples/tests/invalid/class.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,13 @@ class C13 {
z: str;
//^ Symbol "z" is already defined
}

if true {
pub class PublicClass {
// ^ public class "PublicClass" must be declared at the top-level of the file or marked private
}

internal class InternalClass {
// ^ internal class "PublicClass" must be declared at the top-level of the file or marked private
}
}
7 changes: 7 additions & 0 deletions examples/tests/invalid/enums.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,10 @@ let four = SomeEnum.FOUR;

let two = SomeEnum.TWO.TWO;
// ERR ^^^ Property not found

if true {
enum AnotherEnum {
// ^ Enums must be declared at the top-level of the file
FOUR, FIVE, SIX
}
}
9 changes: 8 additions & 1 deletion examples/tests/invalid/interface.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,11 @@ inflight interface IInflightExtendsJsii extends jsii_fixture.ISomeInterface {
// Inflight classes can't implement JSII interfaces
inflight class CInflightImplJsii impl jsii_fixture.ISomeInterface {
pub method(): void {}
}
}

if true {
interface INestedInterface {
// ^ Interfaces must be declared at the top-level of the file
method(): void;
}
}
16 changes: 5 additions & 11 deletions examples/tests/valid/impl_interface.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,11 @@ class Terrier extends Dog {

let w: IAnimal = new Terrier();

// interface inheriting the phase from the function it's inside
// does it make sense to support this?
inflight interface IDog {
bark(): void;
} // All methods should be implicitly inflight

let f = inflight () => {
interface IDog {
bark(): void;
} // All methods should be implicitly inflight
class MyDog impl IDog {
pub bark(): void {
log("woof");
Expand Down Expand Up @@ -108,11 +107,6 @@ class ImplementInflightIfaceInPreflightClass impl IInflight {
}
}

// Extend inflight interface in an inflight interface defined inflight
inflight () => {
interface InflightIfaceDefinedInflight extends IInflight {}
};

// Extend inflight interface in an inflight interface defined preflight
interface InflightIfaceDefinedPreflight extends IInflight {}

Expand All @@ -138,4 +132,4 @@ class ImplPreflightIfaceInPreflightClass impl IPreflight {
pub method() {
return;
}
}
}
33 changes: 32 additions & 1 deletion libs/wingc/src/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3902,7 +3902,7 @@ It should primarily be used in preflight or in inflights that are guaranteed to
if let Some(_) = env.parent {
self.spanned_error(
name,
format!("struct \"{name}\" must be declared at the top-level of a file"),
format!("struct \"{name}\" must be declared at the top-level of the file"),
);
}

Expand Down Expand Up @@ -3959,6 +3959,15 @@ It should primarily be used in preflight or in inflights that are guaranteed to
self.source_file.package.clone(),
);

// Interfaces can only be declared only at the top-level of a program
if let Some(_) = env.parent {
let name = &iface.name;
self.spanned_error(
name,
format!("interface \"{name}\" must be declared at the top-level of the file"),
);
}

// Collect types this interface extends
let extend_interfaces = iface
.extends
Expand Down Expand Up @@ -4010,6 +4019,15 @@ It should primarily be used in preflight or in inflights that are guaranteed to
docs: doc.as_ref().map_or(Docs::default(), |s| Docs::with_summary(s)),
}));

// Enums can only be declared only at the top-level of a program
if let Some(_) = env.parent {
let name = &enu.name;
self.spanned_error(
name,
format!("enum \"{name}\" must be declared at the top-level of the file"),
);
}

match env.define(
&enu.name,
SymbolKind::Type(enum_type_ref),
Expand Down Expand Up @@ -4481,6 +4499,19 @@ It should primarily be used in preflight or in inflights that are guaranteed to
fn type_check_class(&mut self, stmt: &Stmt, ast_class: &AstClass, env: &mut SymbolEnv) {
self.ctx.push_class(ast_class);

// Classes cannot be exported (via "pub" or "internal") if they are
// defined somewhere besides the top-level of the file.
if let Some(_) = env.parent {
let access = ast_class.access;
let name = &ast_class.name;
if access == AccessModifier::Public || access == AccessModifier::Internal {
self.spanned_error(
name,
format!("{access} class \"{name}\" must be declared at the top-level of the file or marked private"),
);
}
}

// preflight classes cannot be declared inside an inflight scope
// (the other way is okay)
if env.phase == Phase::Inflight && ast_class.phase == Phase::Preflight {
Expand Down
32 changes: 30 additions & 2 deletions tools/hangar/__snapshots__/invalid.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,20 @@ error: Expected type to be "str", but got "num" instead
223 | return 5;
| ^


error: public class "PublicClass" must be declared at the top-level of the file or marked private
--> ../../../examples/tests/invalid/class.test.w:231:13
|
231 | pub class PublicClass {
| ^^^^^^^^^^^


error: internal class "InternalClass" must be declared at the top-level of the file or marked private
--> ../../../examples/tests/invalid/class.test.w:235:18
|
235 | internal class InternalClass {
| ^^^^^^^^^^^^^

Tests 1 failed (1)
Snapshots 1 skipped
Test Files 1 failed (1)
Expand Down Expand Up @@ -1481,6 +1495,13 @@ error: Property not found
8 | let two = SomeEnum.TWO.TWO;
| ^^^


error: enum "AnotherEnum" must be declared at the top-level of the file
--> ../../../examples/tests/invalid/enums.test.w:12:8
|
12 | enum AnotherEnum {
| ^^^^^^^^^^^

Tests 1 failed (1)
Snapshots 1 skipped
Test Files 1 failed (1)
Expand Down Expand Up @@ -2827,6 +2848,13 @@ error: Expected type to be "preflight (): void", but got "inflight (): void" ins
|
= hint: expected phase to be preflight, but got inflight instead


error: interface "INestedInterface" must be declared at the top-level of the file
--> ../../../examples/tests/invalid/interface.test.w:72:13
|
72 | interface INestedInterface {
| ^^^^^^^^^^^^^^^^

Tests 1 failed (1)
Snapshots 1 skipped
Test Files 1 failed (1)
Expand Down Expand Up @@ -4861,14 +4889,14 @@ error: "noSuchField" is not a field of "SomeStruct1"
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^


error: struct "PreflightStruct" must be declared at the top-level of a file
error: struct "PreflightStruct" must be declared at the top-level of the file
--> ../../../examples/tests/invalid/structs.test.w:51:10
|
51 | struct PreflightStruct {
| ^^^^^^^^^^^^^^^


error: struct "InflightStruct" must be declared at the top-level of a file
error: struct "InflightStruct" must be declared at the top-level of the file
--> ../../../examples/tests/invalid/structs.test.w:58:10
|
58 | struct InflightStruct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,27 +72,6 @@ module.exports = function({ }) {
//# sourceMappingURL=inflight.$Closure3-1.cjs.map
```
## inflight.$Closure4-1.cjs
```cjs
"use strict";
const $helpers = require("@winglang/sdk/lib/helpers");
const $macros = require("@winglang/sdk/lib/macros");
module.exports = function({ }) {
class $Closure4 {
constructor($args) {
const { } = $args;
const $obj = (...args) => this.handle(...args);
Object.setPrototypeOf($obj, this);
return $obj;
}
async handle() {
}
}
return $Closure4;
}
//# sourceMappingURL=inflight.$Closure4-1.cjs.map
```
## inflight.A-1.cjs
```cjs
"use strict";
Expand Down Expand Up @@ -472,27 +451,6 @@ class $Root extends $stdlib.std.Resource {
});
}
}
class $Closure4 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
super($scope, $id);
$helpers.nodeof(this).hidden = true;
}
static _toInflightType() {
return `
require("${$helpers.normalPath(__dirname)}/inflight.$Closure4-1.cjs")({
})
`;
}
get _liftMap() {
return ({
"handle": [
],
"$inflight_init": [
],
});
}
}
class ImplInflightIfaceInInflightClass extends $stdlib.std.Resource {
constructor($scope, $id, ) {
super($scope, $id);
Expand All @@ -512,8 +470,8 @@ class $Root extends $stdlib.std.Resource {
});
}
}
if ($preflightTypesMap[11]) { throw new Error("ImplInflightIfaceInInflightClass is already in type map"); }
$preflightTypesMap[11] = ImplInflightIfaceInInflightClass;
if ($preflightTypesMap[10]) { throw new Error("ImplInflightIfaceInInflightClass is already in type map"); }
$preflightTypesMap[10] = ImplInflightIfaceInInflightClass;
class ImplInflightIfaceInPreflightClass extends $stdlib.std.Resource {
constructor($scope, $id, ) {
super($scope, $id);
Expand Down Expand Up @@ -564,7 +522,6 @@ class $Root extends $stdlib.std.Resource {
const z = new Dog(this, "Dog");
const w = new Terrier(this, "Terrier");
const f = new $Closure3(this, "$Closure3");
new $Closure4(this, "$Closure4");
}
}
const $APP = $PlatformManager.createApp({ outdir: $outdir, name: "impl_interface.test", rootConstruct: $Root, isTestEnvironment: $wing_is_test, entrypointDir: process.env['WING_SOURCE_DIR'], rootId: process.env['WING_ROOT_ID'] });
Expand Down

0 comments on commit eca0508

Please sign in to comment.