Skip to content

Commit

Permalink
fix(compiler): cannot bring modules more than once (attempt 2) (#3677)
Browse files Browse the repository at this point in the history
Re-roll of the changes in #3605, but with more care taken to support the language server. The previous version of this PR  introduced some bugs where the LSP was using the text from disk instead of the text in the user's editor, resulting in invalid text completions and crashes. To fix this, I had to rethink the original multi-file parsing solution so that we store the latest file text from the LSP in memory.

----

Refactors how the Wing compiler handles `bring` for local Wing files in order to support more complex `bring` use cases, like allowing multiple files to `bring` the same module.

The high level implementation approach is instead of generating a single, monolithic AST, we augment the parser so that after a Wing source file is parsed, an isolated AST and a list of dependent Wing files is produced. Using this, we can write a function that parses an entire Wing program given an entrypoint, and use the `petgraph` crate to produce topological ordering of the files for type checking, and each AST is independently type checked.

Fixes #3511
Fixes #3578
Fixes #3509

## 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)
- [x] 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 7, 2023
1 parent 0ffb623 commit df94899
Show file tree
Hide file tree
Showing 331 changed files with 2,675 additions and 1,490 deletions.
17 changes: 17 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions apps/vscode-wing/.projen/deps.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions apps/vscode-wing/.projen/tasks.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions apps/vscode-wing/.projenrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const project = new TypeScriptAppProject({
"@types/ws",
"@wingconsole/app@workspace:^",
"@wingconsole/server@workspace:^",
"winglang@workspace:^",
],
});

Expand Down
1 change: 1 addition & 0 deletions apps/vscode-wing/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion apps/vscode-wing/turbo.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
"outputs": ["lib/**"]
},
"dev": {
"dependsOn": ["compile"]
"dependsOn": ["compile"],
"cache": false,
"persistent": true
},
"package": {
"inputs": ["syntaxes/**", "resources/**"],
Expand Down
67 changes: 49 additions & 18 deletions apps/wing/src/commands/lsp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,17 @@ export async function lsp() {
let wingc = await wingCompiler.load({
imports: {
env: {
send_diagnostic
send_diagnostic,
},
},
});
let badState = false;

const seenFiles = new Set<DocumentUri>();

const raw_diagnostics: wingCompiler.WingDiagnostic[] = [];

function send_diagnostic(
data_ptr: number,
data_len: number
) {
function send_diagnostic(data_ptr: number, data_len: number) {
const data_buf = Buffer.from(
(wingc.exports.memory as WebAssembly.Memory).buffer,
data_ptr,
Expand Down Expand Up @@ -99,7 +98,11 @@ export async function lsp() {
return result;
});

async function handle_event_and_update_diagnostics(wingc_handler_name: wingCompiler.WingCompilerFunction, params: any, uri: DocumentUri) {
async function handle_event_and_update_diagnostics(
wingc_handler_name: wingCompiler.WingCompilerFunction,
params: any,
_uri: DocumentUri
) {
if (badState) {
wingc = await wingCompiler.load({
imports: {
Expand All @@ -114,24 +117,52 @@ export async function lsp() {
raw_diagnostics.length = 0;
// Call wingc handler
callWing(wingc_handler_name, params);
// purposely not awaiting this, notifications are fire-and-forget
connection.sendDiagnostics({
uri,
diagnostics: raw_diagnostics.map((rd) => {
if(rd.span) {
return Diagnostic.create(Range.create(rd.span.start.line, rd.span.start.col, rd.span.end.line, rd.span.end.col), rd.message)
} else {
return Diagnostic.create(Range.create(0, 0, 0, 0), rd.message)

const allDiagnostics = new Map<DocumentUri, Diagnostic[]>();

// set empty list of diagnostics for files that have been seen before
// this way even if we don't get a diagnostic for a file, we clear out the old ones
for (const uri of seenFiles) {
allDiagnostics.set(uri, []);
}

for (const rd of raw_diagnostics) {
if (rd.span) {
const diagnosticUri = "file://" + rd.span.file_id;
const diag = Diagnostic.create(
Range.create(rd.span.start.line, rd.span.start.col, rd.span.end.line, rd.span.end.col),
rd.message
);

if (!allDiagnostics.has(diagnosticUri)) {
allDiagnostics.set(diagnosticUri, []);
seenFiles.add(diagnosticUri);
}
})
});
allDiagnostics.get(diagnosticUri)!.push(diag);
} else {
// skip if diagnostic is not associated with any file
}
}

// purposely not awaiting these calls, notifications are fire-and-forget
for (const [uri, diagnostics] of allDiagnostics.entries()) {
connection.sendDiagnostics({ uri, diagnostics });
}
}

connection.onDidOpenTextDocument(async (params) => {
handle_event_and_update_diagnostics("wingc_on_did_open_text_document", params, params.textDocument.uri);
handle_event_and_update_diagnostics(
"wingc_on_did_open_text_document",
params,
params.textDocument.uri
);
});
connection.onDidChangeTextDocument(async (params) => {
handle_event_and_update_diagnostics("wingc_on_did_change_text_document", params, params.textDocument.uri);
handle_event_and_update_diagnostics(
"wingc_on_did_change_text_document",
params,
params.textDocument.uri
);
});
connection.onCompletion(async (params) => {
return callWing("wingc_on_completion", params);
Expand Down
3 changes: 3 additions & 0 deletions examples/tests/invalid/bring_local_self.w
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
bring "./bring_local_self.w" as foo;
// ^ error: Cannot bring a module into itself

bring "./non-existent.w" as bar;
// ^ error: Could not find Wing module "./non-existent.w"
1 change: 1 addition & 0 deletions examples/tests/invalid/cyclic_bring1.w
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bring "./cyclic_bring2.w" as foo;
1 change: 1 addition & 0 deletions examples/tests/invalid/cyclic_bring2.w
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bring "./cyclic_bring3.w" as foo;
1 change: 1 addition & 0 deletions examples/tests/invalid/cyclic_bring3.w
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bring "./cyclic_bring1.w" as foo;
7 changes: 7 additions & 0 deletions examples/tests/valid/baz.w
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// used by bring_local_normalization.w

class Baz {
static baz(): str {
return "baz";
}
}
1 change: 1 addition & 0 deletions examples/tests/valid/bring_local.w
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
bring "./store.w" as file1;
bring "./subdir/subfile.w" as file2;
bring "./subdir/empty.w" as file3;
bring math;

// classes from other files can be used
let store = new file1.Store();
Expand Down
13 changes: 13 additions & 0 deletions examples/tests/valid/bring_local_normalization.w
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
bring "./subdir/foo.w" as foo;
bring "./subdir/bar.w" as bar;
bring "./baz.w" as baz;

// in this app, "bar" is imported two ways (here as "./subdir/bar.w" and in foo.w as "./bar.w")
// and "baz" is imported two ways (here as "./baz.w" and in foo.w as "../baz.w")
// this test checks that the different imports all resolve to the same module

assert(foo.Foo.foo() == "foo");
assert(foo.Foo.bar() == "bar");
assert(foo.Foo.baz() == "baz");
assert(bar.Bar.bar() == "bar");
assert(baz.Baz.baz() == "baz");
3 changes: 3 additions & 0 deletions examples/tests/valid/store.w
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
bring "./subdir/empty.w" as file3;
bring math;

bring cloud;

class Store {
Expand Down
7 changes: 7 additions & 0 deletions examples/tests/valid/subdir/bar.w
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// used by bring_local_normalization.w

class Bar {
static bar(): str {
return "bar";
}
}
16 changes: 16 additions & 0 deletions examples/tests/valid/subdir/foo.w
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// used by bring_local_normalization.w

bring "./bar.w" as bar;
bring "../baz.w" as baz;

class Foo {
static foo(): str {
return "foo";
}
static bar(): str {
return bar.Bar.bar();
}
static baz(): str {
return baz.Baz.baz();
}
}
2 changes: 2 additions & 0 deletions examples/tests/valid/subdir/subfile.w
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
bring math;

class Q {}
1 change: 1 addition & 0 deletions libs/wingc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ indoc = "2.0.0"
const_format = "0.2.30"
duplicate = "1.0.0"
strum = { version = "0.24", features = ["derive"] }
petgraph = "0.6.3"

[lib]
crate-type = ["rlib", "cdylib"]
Expand Down
18 changes: 13 additions & 5 deletions libs/wingc/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,16 +421,19 @@ pub struct Interface {
pub extends: Vec<UserDefinedType>,
}

#[derive(Debug)]
pub enum BringSource {
BuiltinModule(Symbol),
JsiiModule(Symbol),
WingFile(Symbol),
}

#[derive(Debug)]
pub enum StmtKind {
Bring {
module_name: Symbol, // Reference?
source: BringSource,
identifier: Option<Symbol>,
},
Module {
name: Symbol,
statements: Scope,
},
SuperConstructor {
arg_list: ArgList,
},
Expand Down Expand Up @@ -675,6 +678,11 @@ impl Scope {
assert!((*env).is_none());
*env = Some(new_env);
}

pub fn reset_env(&self) {
let mut env = self.env.borrow_mut();
*env = None;
}
}

#[derive(Debug)]
Expand Down
Loading

0 comments on commit df94899

Please sign in to comment.