From e20445549974b658bcb5372f041f2bb3339d79bd Mon Sep 17 00:00:00 2001 From: Microsoft Auto Changeset Bot Date: Tue, 5 Sep 2023 11:02:01 -0700 Subject: [PATCH 1/4] Alias of namespace --- packages/compiler/src/core/checker.ts | 1 + packages/compiler/test/checker/alias.test.ts | 28 +++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index fbdbfbb0fd9..1e8292cb64a 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -3944,6 +3944,7 @@ export function createChecker(program: Program): Checker { if (targetBinding.flags & SymbolFlags.Namespace) { mergedSymbols.set(sourceBinding, targetBinding); mutate(targetBinding.declarations).push(...sourceBinding.declarations); + mutate(targetBinding.declarations).sort((a, b) => b.kind - a.kind); // Makes sure that Namespace Node are before JsSourceFile declarations. mergeSymbolTable(sourceBinding.exports!, mutate(targetBinding.exports!)); } else { // this will set a duplicate error diff --git a/packages/compiler/test/checker/alias.test.ts b/packages/compiler/test/checker/alias.test.ts index ef239308b3a..3f25b5cda4d 100644 --- a/packages/compiler/test/checker/alias.test.ts +++ b/packages/compiler/test/checker/alias.test.ts @@ -1,6 +1,11 @@ import { ok, strictEqual } from "assert"; import { Model, Type, Union } from "../../src/core/types.js"; -import { TestHost, createTestHost, expectDiagnostics } from "../../src/testing/index.js"; +import { + TestHost, + createTestHost, + expectDiagnosticEmpty, + expectDiagnostics, +} from "../../src/testing/index.js"; describe("compiler: aliases", () => { let testHost: TestHost; @@ -231,4 +236,25 @@ describe("compiler: aliases", () => { message: "Alias type 'A' recursively references itself.", }); }); + + // REGRESSION TEST: https://github.com/Azure/typespec-azure/issues/3365 + it("alias an namespace in JS file shouldn't crash", async () => { + testHost.addJsFile("lib.js", { + namespace: "Foo.Bar", + $foo: () => {}, + }); + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./lib.js"; + namespace Foo.Bar { op abc(): void;} + + alias Aliased = Foo.Bar; + op getSmurf is Aliased.abc; + + ` + ); + const diagnostics = await testHost.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); }); From a91769be408ae110eac3424334fc0c4ddf8b12e3 Mon Sep 17 00:00:00 2001 From: Microsoft Auto Changeset Bot Date: Tue, 5 Sep 2023 11:03:07 -0700 Subject: [PATCH 2/4] Changelog --- ...ompiler-crash-alias-namespace_2023-09-05-18-03.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@typespec/compiler/fix-compiler-crash-alias-namespace_2023-09-05-18-03.json diff --git a/common/changes/@typespec/compiler/fix-compiler-crash-alias-namespace_2023-09-05-18-03.json b/common/changes/@typespec/compiler/fix-compiler-crash-alias-namespace_2023-09-05-18-03.json new file mode 100644 index 00000000000..a2e39296c4f --- /dev/null +++ b/common/changes/@typespec/compiler/fix-compiler-crash-alias-namespace_2023-09-05-18-03.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/compiler", + "comment": "**Fix** Compiler crash when using alias of namespace that has decorators", + "type": "none" + } + ], + "packageName": "@typespec/compiler" +} \ No newline at end of file From 6411b5143245a7f919897ff5a18985dc1abbcf20 Mon Sep 17 00:00:00 2001 From: Microsoft Auto Changeset Bot Date: Tue, 5 Sep 2023 11:49:59 -0700 Subject: [PATCH 3/4] Create a virtual JS namespace node --- packages/compiler/src/core/binder.ts | 55 +++++++++++++------ packages/compiler/src/core/checker.ts | 24 ++++---- packages/compiler/src/core/parser.ts | 1 + packages/compiler/src/core/types.ts | 8 ++- .../compiler/src/formatter/print/printer.ts | 1 + 5 files changed, 62 insertions(+), 27 deletions(-) diff --git a/packages/compiler/src/core/binder.ts b/packages/compiler/src/core/binder.ts index 030b36bdb92..156e2aea857 100644 --- a/packages/compiler/src/core/binder.ts +++ b/packages/compiler/src/core/binder.ts @@ -1,5 +1,10 @@ import { compilerAssert } from "./diagnostics.js"; -import { FileLibraryMetadata, getLocationContext } from "./index.js"; +import { + FileLibraryMetadata, + JsNamespaceDeclarationNode, + NodeFlags, + getLocationContext, +} from "./index.js"; import { visitChildren } from "./parser.js"; import { Program } from "./program.js"; import { @@ -154,18 +159,27 @@ export function createBinder(program: Program): Binder { kind = "function"; } - const memberNs: string = (member as any).namespace; - const nsParts = []; - if (rootNs) { - nsParts.push(...rootNs.split(".")); - } - - if (memberNs) { - nsParts.push(...memberNs.split(".")); - } - + const nsParts = resolveJSMemberNamespaceParts(rootNs, member); for (const part of nsParts) { const existingBinding = containerSymbol.exports!.get(part); + const jsNamespaceNode: JsNamespaceDeclarationNode = { + kind: SyntaxKind.JsNamespaceDeclaration, + id: { + kind: SyntaxKind.Identifier, + sv: part, + pos: 0, + end: 0, + flags: NodeFlags.None, + symbol: undefined!, + }, + pos: sourceFile.pos, + end: sourceFile.end, + parent: sourceFile, + flags: NodeFlags.None, + symbol: undefined!, + }; + const sym = createSymbol(jsNamespaceNode, part, SymbolFlags.Namespace, containerSymbol); + mutate(jsNamespaceNode).symbol = sym; if (existingBinding) { if (existingBinding.flags & SymbolFlags.Namespace) { // since the namespace was "declared" as part of this source file, @@ -173,13 +187,9 @@ export function createBinder(program: Program): Binder { containerSymbol = existingBinding; } else { // we have some conflict, lets report a duplicate binding error. - mutate(containerSymbol.exports)!.set( - part, - createSymbol(sourceFile, part, SymbolFlags.Namespace, containerSymbol) - ); + mutate(containerSymbol.exports)!.set(part, sym); } } else { - const sym = createSymbol(sourceFile, part, SymbolFlags.Namespace, containerSymbol); mutate(sym).exports = createSymbolTable(); mutate(containerSymbol.exports!).set(part, sym); containerSymbol = sym; @@ -212,6 +222,19 @@ export function createBinder(program: Program): Binder { } } + function resolveJSMemberNamespaceParts(rootNs: string | undefined, member: any) { + const memberNs: string = member.namespace; + const nsParts = []; + if (rootNs) { + nsParts.push(...rootNs.split(".")); + } + + if (memberNs) { + nsParts.push(...memberNs.split(".")); + } + return nsParts; + } + function bindSourceFile(script: TypeSpecScriptNode) { if (script.locals !== undefined) { return; diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index 1e8292cb64a..d58c22ab9d4 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -48,6 +48,7 @@ import { InterfaceStatementNode, IntersectionExpressionNode, IntrinsicScalarName, + JsNamespaceDeclarationNode, JsSourceFileNode, LiteralNode, LiteralType, @@ -632,6 +633,7 @@ export function createChecker(program: Program): Checker { case SyntaxKind.UnionVariant: return checkUnionVariant(node, mapper); case SyntaxKind.NamespaceStatement: + case SyntaxKind.JsNamespaceDeclaration: return checkNamespace(node); case SyntaxKind.OperationStatement: return checkOperation(node, mapper); @@ -1479,23 +1481,25 @@ export function createChecker(program: Program): Checker { return getOrInstantiateTemplate(arrayNode, [param], [elementType], undefined) as Model; } - function checkNamespace(node: NamespaceStatementNode) { + function checkNamespace(node: NamespaceStatementNode | JsNamespaceDeclarationNode) { const links = getSymbolLinks(getMergedSymbol(node.symbol)); let type = links.type as Namespace; if (!type) { type = initializeTypeForNamespace(node); } - if (isArray(node.statements)) { - node.statements.forEach((x) => getTypeForNode(x)); - } else if (node.statements) { - const subNs = checkNamespace(node.statements); - type.namespaces.set(subNs.name, subNs); + if (node.kind === SyntaxKind.NamespaceStatement) { + if (isArray(node.statements)) { + node.statements.forEach((x) => getTypeForNode(x)); + } else if (node.statements) { + const subNs = checkNamespace(node.statements); + type.namespaces.set(subNs.name, subNs); + } } return type; } - function initializeTypeForNamespace(node: NamespaceStatementNode) { + function initializeTypeForNamespace(node: NamespaceStatementNode | JsNamespaceDeclarationNode) { compilerAssert(node.symbol, "Namespace is unbound.", node); const mergedSymbol = getMergedSymbol(node.symbol)!; const symbolLinks = getSymbolLinks(mergedSymbol); @@ -1507,7 +1511,7 @@ export function createChecker(program: Program): Checker { kind: "Namespace", name, namespace, - node, + node: node, models: new Map(), scalars: new Map(), operations: new Map(), @@ -1539,6 +1543,7 @@ export function createChecker(program: Program): Checker { | ModelStatementNode | ScalarStatementNode | NamespaceStatementNode + | JsNamespaceDeclarationNode | OperationStatementNode | EnumStatementNode | InterfaceStatementNode @@ -2294,7 +2299,7 @@ export function createChecker(program: Program): Checker { default: // get the symbol from the node aliased type's node, or just return the base // if it doesn't have a symbol (which will likely result in an error later on) - return aliasType.node!.symbol ?? aliasSymbol; + return getMergedSymbol(aliasType.node!.symbol) ?? aliasSymbol; } } function checkStringLiteral(str: StringLiteralNode): StringLiteral { @@ -3944,7 +3949,6 @@ export function createChecker(program: Program): Checker { if (targetBinding.flags & SymbolFlags.Namespace) { mergedSymbols.set(sourceBinding, targetBinding); mutate(targetBinding.declarations).push(...sourceBinding.declarations); - mutate(targetBinding.declarations).sort((a, b) => b.kind - a.kind); // Makes sure that Namespace Node are before JsSourceFile declarations. mergeSymbolTable(sourceBinding.exports!, mutate(targetBinding.exports!)); } else { // this will set a duplicate error diff --git a/packages/compiler/src/core/parser.ts b/packages/compiler/src/core/parser.ts index b3f03ab638b..538cc4c04e5 100644 --- a/packages/compiler/src/core/parser.ts +++ b/packages/compiler/src/core/parser.ts @@ -3148,6 +3148,7 @@ export function visitChildren(node: Node, cb: NodeCallback): T | undefined case SyntaxKind.ExternKeyword: case SyntaxKind.UnknownKeyword: case SyntaxKind.JsSourceFile: + case SyntaxKind.JsNamespaceDeclaration: case SyntaxKind.DocText: return; diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index 32f767041ca..51d6668305a 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -390,7 +390,7 @@ export interface Namespace extends BaseType, DecoratedType { kind: "Namespace"; name: string; namespace?: Namespace; - node: NamespaceStatementNode; + node: NamespaceStatementNode | JsNamespaceDeclarationNode; /** * The models in the namespace. @@ -783,6 +783,7 @@ export enum SyntaxKind { ProjectionStatement, ProjectionDecoratorReferenceExpression, Return, + JsNamespaceDeclaration, } export const enum NodeFlags { @@ -838,6 +839,7 @@ export interface TemplateDeclarationNode { export type Node = | TypeSpecScriptNode | JsSourceFileNode + | JsNamespaceDeclarationNode | TemplateParameterDeclarationNode | ProjectionParameterDeclarationNode | ProjectionLambdaParameterDeclarationNode @@ -1616,6 +1618,10 @@ export interface JsSourceFileNode extends DeclarationNode, BaseNode { readonly namespaceSymbols: Sym[]; } +export interface JsNamespaceDeclarationNode extends DeclarationNode, BaseNode { + readonly kind: SyntaxKind.JsNamespaceDeclaration; +} + export type EmitterFunc = (context: EmitContext) => Promise | void; export interface SourceFile { diff --git a/packages/compiler/src/formatter/print/printer.ts b/packages/compiler/src/formatter/print/printer.ts index ac1febd43dc..7acb3513f1b 100644 --- a/packages/compiler/src/formatter/print/printer.ts +++ b/packages/compiler/src/formatter/print/printer.ts @@ -357,6 +357,7 @@ export function printNode( case SyntaxKind.EmptyStatement: return ""; case SyntaxKind.JsSourceFile: + case SyntaxKind.JsNamespaceDeclaration: case SyntaxKind.InvalidStatement: return getRawText(node, options); default: From 61c685fdfa5965a2f8f076aab2b5095c954bc812 Mon Sep 17 00:00:00 2001 From: Microsoft Auto Changeset Bot Date: Tue, 5 Sep 2023 12:40:46 -0700 Subject: [PATCH 4/4] Works --- packages/compiler/src/core/checker.ts | 3 ++- packages/compiler/test/binder.test.ts | 4 ++-- packages/compiler/test/server/completion.test.ts | 5 ++++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index d58c22ab9d4..352f7c54075 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -1607,7 +1607,8 @@ export function createChecker(program: Program): Checker { // refers to a model in another namespace. In this case, we need to evaluate // the namespace here. const namespaceNode = mergedSymbol.declarations.find( - (x): x is NamespaceStatementNode => x.kind === SyntaxKind.NamespaceStatement + (x): x is NamespaceStatementNode => + x.kind === SyntaxKind.NamespaceStatement || x.kind === SyntaxKind.JsNamespaceDeclaration ); compilerAssert(namespaceNode, "Can't find namespace declaration node.", node); symbolLinks.type = initializeTypeForNamespace(namespaceNode); diff --git a/packages/compiler/test/binder.test.ts b/packages/compiler/test/binder.test.ts index a1b4b9efa62..ed19e0fffa7 100644 --- a/packages/compiler/test/binder.test.ts +++ b/packages/compiler/test/binder.test.ts @@ -390,11 +390,11 @@ describe("compiler: binder", () => { assertBindings("jsFile", sourceFile.symbol.exports!, { Foo: { flags: SymbolFlags.Namespace, - declarations: [SyntaxKind.JsSourceFile], + declarations: [SyntaxKind.JsNamespaceDeclaration], exports: { Bar: { flags: SymbolFlags.Namespace, - declarations: [SyntaxKind.JsSourceFile], + declarations: [SyntaxKind.JsNamespaceDeclaration], exports: { "@myDec2": { flags: SymbolFlags.Decorator | SymbolFlags.Implementation, diff --git a/packages/compiler/test/server/completion.test.ts b/packages/compiler/test/server/completion.test.ts index 60ec5752329..91e4365fb2c 100644 --- a/packages/compiler/test/server/completion.test.ts +++ b/packages/compiler/test/server/completion.test.ts @@ -675,7 +675,10 @@ describe("compiler: server: completion", () => { label: "Inner", insertText: "Inner", kind: CompletionItemKind.Module, - documentation: undefined, + documentation: { + kind: MarkupKind.Markdown, + value: "```typespec\nnamespace Outer.Inner\n```", + }, }, { label: "outerDecorator",