Outline view improvements (#233)

* Add unit tests * Fix unnamed modules * Add type definitions to outline / go-to-symbol menu * Change outline view icon for consistency with autocompletion * Progress * Simplify unit tests * Progress * Progress * Fix class DecField parsing * Handle implicit actor async/await AST nodes * Misc * Remove temporary comment * Account for type bindings in 'ClassD' * Show class fields in outline view
dfinity · Aug 15, 2023 · f6139cc · f6139cc
1 parent 1ec8213
commit f6139cc
Show file tree

Hide file tree

Showing 5 changed files with 228 additions and 42 deletions.
diff --git a/package.json b/package.json
@@ -215,7 +215,7 @@
         "compile:server": "esbuild ./src/server/server.ts --bundle --outfile=out/server.js --external:vscode --format=cjs --platform=node --minify",
         "compile:motoko": "esbuild motoko --bundle --outfile=out/motoko.js --format=cjs --platform=node --minify",
         "test": "jest",
-        "package": "vsce package && npm test",
+        "package": "vsce package && npm test && npm run --silent lint",
         "publish": "vsce publish"
     },
     "lint-staged": {

diff --git a/src/server/imports.ts b/src/server/imports.ts
@@ -35,7 +35,7 @@ export default class ImportResolver {
         this._fileSystemMap.set(importUri, uri);
         if (program?.export) {
             // Resolve field names
-            const ast = program.export;
+            const { ast } = program.export;
             const node =
                 matchNode(ast, 'LetD', (_pat: Node, exp: Node) => exp) || // Named
                 matchNode(ast, 'ExpD', (exp: Node) => exp); // Unnamed

diff --git a/src/server/server.ts b/src/server/server.ts
@@ -60,7 +60,16 @@ import {
     rangeFromNode,
 } from './navigation';
 import { deployPlayground } from './playground';
-import { Field, ObjBlock, Program, asNode, findNodes } from './syntax';
+import {
+    Class,
+    Field,
+    ObjBlock,
+    Program,
+    SyntaxWithFields,
+    Type,
+    asNode,
+    findNodes,
+} from './syntax';
 import {
     formatMotoko,
     getFileText,
@@ -1214,8 +1223,10 @@ connection.onWorkspaceSymbol((event) => {
         symbol.children?.forEach((s) => visitDocumentSymbol(uri, s, symbol));
     };
     globalASTCache.forEach((status) => {
-        status.program?.namedExports.forEach((field) => {
-            visitDocumentSymbol(status.uri, getDocumentSymbol(field));
+        status.program?.exportFields.forEach((field) => {
+            getDocumentSymbols(field, true).forEach((symbol) =>
+                visitDocumentSymbol(status.uri, symbol),
+            );
         });
     });
     return results;
@@ -1225,29 +1236,47 @@ connection.onDocumentSymbol((event) => {
     const { uri } = event.textDocument;
     const results: DocumentSymbol[] = [];
     const status = getContext(uri).astResolver.request(uri);
-    status?.program?.namedExports.forEach((field) => {
-        results.push(getDocumentSymbol(field));
+    status?.program?.exportFields.forEach((field) => {
+        results.push(...getDocumentSymbols(field, false));
     });
     return results;
 });
 
-function getDocumentSymbol(field: Field): DocumentSymbol {
+function getDocumentSymbols(
+    field: Field,
+    skipUnnamed: boolean,
+): DocumentSymbol[] {
     const range = rangeFromNode(asNode(field.ast)) || defaultRange();
     const kind =
-        field.exp instanceof ObjBlock ? SymbolKind.Module : SymbolKind.Field;
+        field.exp instanceof ObjBlock
+            ? SymbolKind.Module
+            : field.exp instanceof Class
+            ? SymbolKind.Class
+            : field.exp instanceof Type
+            ? SymbolKind.Interface
+            : SymbolKind.Variable;
     const children: DocumentSymbol[] = [];
-    if (field.exp instanceof ObjBlock) {
+    if (field.exp instanceof SyntaxWithFields) {
         field.exp.fields.forEach((field) => {
-            children.push(getDocumentSymbol(field));
+            children.push(...getDocumentSymbols(field, skipUnnamed));
         });
     }
-    return {
-        name: field.name,
-        kind,
-        range,
-        selectionRange: rangeFromNode(asNode(field.pat?.ast)) || range,
-        children,
-    };
+    if (skipUnnamed && !field.name) {
+        return children;
+    }
+    return [
+        {
+            name:
+                field.name ||
+                (field.exp instanceof ObjBlock
+                    ? field.exp.sort.toLowerCase()
+                    : '(unknown)'), // Default field name
+            kind,
+            range,
+            selectionRange: rangeFromNode(asNode(field.pat?.ast)) || range,
+            children,
+        },
+    ];
 }
 
 connection.onReferences(

diff --git a/src/server/syntax.spec.ts b/src/server/syntax.spec.ts
@@ -0,0 +1,93 @@
+import motoko from './motoko';
+import { Field, Program, Syntax, SyntaxWithFields, fromAST } from './syntax';
+
+/* eslint jest/expect-expect: ["off", { "assertFunctionNames": ["expect"] }] */
+
+const parse = (source: string): Program => {
+    const ast = motoko.parseMotoko(source);
+    const prog = fromAST(ast) as Program;
+    expect(prog).toBeInstanceOf(Program);
+    return prog;
+};
+
+const expectFields = (
+    fields: Field[],
+    expected: (string | undefined)[],
+): void => {
+    expect(fields.map((f) => f.name)).toStrictEqual(expected);
+};
+
+const expectWithFields = (
+    syntax: Syntax,
+    expected: (string | undefined)[],
+): Field[] => {
+    const obj = syntax as SyntaxWithFields;
+    expect(obj).toBeInstanceOf(SyntaxWithFields);
+    expectFields(obj.fields, expected);
+    return obj.fields;
+};
+
+describe('syntax', () => {
+    test('let field', () => {
+        const prog = parse('module { let x = 0; }');
+        expectWithFields(prog.exportFields[0].exp, ['x']);
+    });
+    test('public let field', () => {
+        const prog = parse('module { public let x = 0; }');
+        expectWithFields(prog.exportFields[0].exp, ['x']);
+    });
+    test('var field', () => {
+        const prog = parse('module { var y = 1; }');
+        expectWithFields(prog.exportFields[0].exp, ['y']);
+    });
+    test('type field', () => {
+        const prog = parse('module { type T = Nat; }');
+        expectWithFields(prog.exportFields[0].exp, ['T']);
+    });
+    test('multiple fields', () => {
+        const prog = parse('module { let x = 0; var y = 1; type T = Nat; }');
+        expectWithFields(prog.exportFields[0].exp, ['x', 'y', 'T']);
+    });
+    test('named actor', () => {
+        const prog = parse('actor A { let x = 0; }');
+        expectFields(prog.exportFields, ['A']);
+        expectWithFields(prog.exportFields[0].exp, ['x']);
+    });
+    test('unnamed actor', () => {
+        const prog = parse('actor { let x = 0; }');
+        expectFields(prog.exportFields, [undefined]);
+        expectWithFields(prog.exportFields[0].exp, ['x']);
+    });
+    test('named class', () => {
+        const prog = parse('class C() { let x = 0; }');
+        expectFields(prog.exportFields, ['C']);
+        expectWithFields(prog.exportFields[0].exp, ['x']);
+    });
+    test('named actor class', () => {
+        const prog = parse('actor class C() { stable var y = 1; }');
+        expectFields(prog.exportFields, ['C']);
+        expectWithFields(prog.exportFields[0].exp, ['y']);
+    });
+    test('named module', () => {
+        const prog = parse('module M { let x = 0; }');
+        expectFields(prog.exportFields, ['M']);
+        expectWithFields(prog.exportFields[0].exp, ['x']);
+    });
+    test('unnamed module', () => {
+        const prog = parse('module { let x = 0; }');
+        expectFields(prog.exportFields, [undefined]);
+        expectWithFields(prog.exportFields[0].exp, ['x']);
+    });
+    test('nested module', () => {
+        const prog = parse('module M { module N { let x = 0; } }');
+        expectFields(prog.exportFields, ['M']);
+        const fields = expectWithFields(prog.exportFields[0].exp, ['N']);
+        expectWithFields(fields[0].exp, ['x']);
+    });
+    test('nested unnamed module', () => {
+        const prog = parse('module { module { let x = 0; } }');
+        expectFields(prog.exportFields, [undefined]);
+        const fields = expectWithFields(prog.exportFields[0].exp, [undefined]);
+        expectWithFields(fields[0].exp, ['x']);
+    });
+});
diff --git a/src/server/syntax.ts b/src/server/syntax.ts
@@ -47,6 +47,12 @@ export function fromAST(ast: AST): Syntax {
         typeof ast === 'number'
     ) {
         return new Syntax(ast);
+    } else if (ast.name === 'AwaitE') {
+        const exp = ast.args![0];
+        return (
+            matchNode(exp, 'AsyncE', (_id: Node, exp: Node) => fromAST(exp)) ||
+            new Syntax(exp)
+        );
     } else if (ast.name === 'Prog') {
         const prog = new Program(ast);
         if (ast.args) {
@@ -80,8 +86,8 @@ export function fromAST(ast: AST): Syntax {
             if (ast.args.length) {
                 const export_ = ast.args[ast.args.length - 1];
                 if (export_) {
-                    prog.export = export_;
-                    prog.namedExports.push(...getFieldsFromAST(export_));
+                    prog.export = fromAST(export_);
+                    prog.exportFields.push(...getFieldsFromAST(export_));
                 }
             }
         }
@@ -106,23 +112,75 @@ export function fromAST(ast: AST): Syntax {
             obj.fields.push(...getFieldsFromAST(dec));
         });
         return obj;
-    } else {
-        return new Syntax(ast);
     }
+    return new Syntax(ast);
 }
 
 function getFieldsFromAST(ast: AST): Field[] {
-    const fields: [string, Node, Node][] =
-        matchNode(ast, 'LetD', (pat: Node, exp: Node) => {
-            const name = matchNode(pat, 'VarP', (field: string) => field);
-            return name ? [[name, pat, exp]] : undefined;
-        }) || [];
-    return fields.map(([name, pat, exp]) => {
-        const field = new Field(ast, name);
-        field.pat = fromAST(pat);
-        field.exp = fromAST(exp);
-        return field;
-    });
+    const simplyNamedFields =
+        matchNode(ast, 'TypD', (name: string, type: Node) => {
+            const field = new Field(ast, new Type(type));
+            field.name = name;
+            return [field];
+        }) ||
+        matchNode(ast, 'VarD', (name: string, exp: Node) => {
+            const field = new Field(ast, new Type(exp));
+            field.name = name;
+            return [field];
+        }) ||
+        matchNode(
+            ast,
+            'ClassD',
+            (_sharedPat: any, name: string, ...args: any[]) => {
+                let index = args.length - 1;
+                while (index >= 0 && typeof args[index] !== 'string') {
+                    index--;
+                }
+                index -= 3; // [pat, returnType, sort]
+                if (index < 0) {
+                    console.warn('Unexpected `ClassD` AST format');
+                    return [];
+                }
+                // const typeBinds = args.slice(0, index) as Node[];
+                const [_pat, _returnType, sort, _id, ...decs] = args.slice(
+                    index,
+                ) as [Node, Node, ObjSort, string, ...Node[]];
+
+                const cls = new Class(ast, name, sort);
+                decs.forEach((ast) => {
+                    matchNode(ast, 'DecField', (dec: Node) => {
+                        cls.fields.push(...getFieldsFromAST(dec));
+                    });
+                });
+                const field = new Field(ast, cls);
+                field.name = name;
+                return [field];
+            },
+        );
+    if (simplyNamedFields) {
+        return simplyNamedFields;
+    }
+    const parts: [Node | undefined, Node] | undefined =
+        matchNode(ast, 'LetD', (pat: Node, exp: Node) => [pat, exp]) || // Named
+        matchNode(ast, 'ExpD', (exp: Node) => [undefined, exp]); // Unnamed
+    if (!parts) {
+        return [];
+    }
+    const [pat, exp] = parts;
+    if (pat) {
+        // TODO: object patterns
+        const fields: [string, Node, Node][] =
+            matchNode(pat, 'VarP', (name: string) => [[name, pat, exp]]) || [];
+        return fields.map(([name, pat, exp]) => {
+            const field = new Field(ast, fromAST(exp));
+            field.name = name;
+            field.pat = fromAST(pat);
+            return field;
+        });
+    } else {
+        const field = new Field(ast, fromAST(exp));
+        return [field];
+    }
 }
 
 export function asNode(ast: AST | undefined): Node | undefined {
@@ -158,25 +216,33 @@ export class Syntax {
 
 export class Program extends Syntax {
     imports: Import[] = [];
-    namedExports: Field[] = [];
-    export: AST | undefined;
+    export: Syntax | undefined;
+    exportFields: Field[] = [];
 }
 
-export type ObjSort = 'Object' | 'Actor' | 'Module' | 'Memory';
-
-export class ObjBlock extends Syntax {
+export abstract class SyntaxWithFields extends Syntax {
     fields: Field[] = [];
+}
 
+export type ObjSort = 'Object' | 'Actor' | 'Module' | 'Memory';
+
+export class ObjBlock extends SyntaxWithFields {
     constructor(ast: AST, public sort: ObjSort) {
         super(ast);
     }
 }
 
+export class Class extends SyntaxWithFields {
+    constructor(ast: AST, public name: string, public sort: ObjSort) {
+        super(ast);
+    }
+}
+
 export class Field extends Syntax {
+    name: string | undefined;
     pat: Syntax | undefined;
-    exp: Syntax | undefined;
 
-    constructor(ast: AST, public name: string) {
+    constructor(ast: AST, public exp: Syntax) {
         super(ast);
     }
 }
@@ -190,6 +256,4 @@ export class Import extends Syntax {
     }
 }
 
-export class Expression extends Syntax {}
-
 export class Type extends Syntax {}