Fix: Augment decorators might not run if alias caused the type to be resolved too early

common/changes/@typespec/compiler/fix-augment-symbol-already-checked_2023-10-25-00-16.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@typespec/compiler",
+      "comment": "Fix: Issue where referencing a template in an alias might cause augment decorators to not be applied on types referenced in the aliased type.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@typespec/compiler"
+}

packages/compiler/src/core/checker.ts

@@ -1858,14 +1858,14 @@ export function createChecker(program: Program): Checker {
   function resolveIdentifierInTable(
     node: IdentifierNode,
     table: SymbolTable | undefined,
-    resolveDecorator = false
+    options: SymbolResolutionOptions
   ): Sym | undefined {
     if (!table) {
       return undefined;
     }
     table = augmentedSymbolTables.get(table) ?? table;
     let sym;
-    if (resolveDecorator) {
+    if (options.resolveDecorators) {
       sym = table.get("@" + node.sv);
     } else {
       sym = table.get(node.sv);
@@ -1921,7 +1921,11 @@ export function createChecker(program: Program): Checker {
         }
 
         lateBindMembers(containerType, container);
-        sym = resolveIdentifierInTable(id, container.exports ?? container.members);
+        sym = resolveIdentifierInTable(
+          id,
+          container.exports ?? container.members,
+          defaultSymbolResolutionOptions
+        );
         break;
       case IdentifierKind.Other:
         return undefined;
@@ -1976,7 +1980,7 @@ export function createChecker(program: Program): Checker {
       let base = resolveTypeReferenceSym(identifier.parent.base, undefined, false);
       if (base) {
         if (base.flags & SymbolFlags.Alias) {
-          base = getAliasedSymbol(base, undefined);
+          base = getAliasedSymbol(base, undefined, defaultSymbolResolutionOptions);
         }
         if (base) {
           if (isTemplatedNode(base.declarations[0])) {
@@ -2075,7 +2079,7 @@ export function createChecker(program: Program): Checker {
   function resolveIdentifierInScope(
     node: IdentifierNode,
     mapper: TypeMapper | undefined,
-    resolveDecorator = false
+    options: SymbolResolutionOptions
   ): Sym | undefined {
     compilerAssert(
       node.parent?.kind !== SyntaxKind.MemberExpression || node.parent.id !== node,
@@ -2094,12 +2098,12 @@ export function createChecker(program: Program): Checker {
     while (scope && scope.kind !== SyntaxKind.TypeSpecScript) {
       if (scope.symbol && "exports" in scope.symbol) {
         const mergedSymbol = getMergedSymbol(scope.symbol);
-        binding = resolveIdentifierInTable(node, mergedSymbol.exports, resolveDecorator);
+        binding = resolveIdentifierInTable(node, mergedSymbol.exports, options);
         if (binding) return binding;
       }
 
       if ("locals" in scope) {
-        binding = resolveIdentifierInTable(node, scope.locals, resolveDecorator);
+        binding = resolveIdentifierInTable(node, scope.locals, options);
         if (binding) return binding;
       }
 
@@ -2110,7 +2114,7 @@ export function createChecker(program: Program): Checker {
       // check any blockless namespace decls
       for (const ns of scope.inScopeNamespaces) {
         const mergedSymbol = getMergedSymbol(ns.symbol);
-        binding = resolveIdentifierInTable(node, mergedSymbol.exports, resolveDecorator);
+        binding = resolveIdentifierInTable(node, mergedSymbol.exports, options);
 
         if (binding) return binding;
       }
@@ -2119,11 +2123,11 @@ export function createChecker(program: Program): Checker {
       const globalBinding = resolveIdentifierInTable(
         node,
         globalNamespaceNode.symbol.exports,
-        resolveDecorator
+        options
       );
 
       // check using types
-      const usingBinding = resolveIdentifierInTable(node, scope.locals, resolveDecorator);
+      const usingBinding = resolveIdentifierInTable(node, scope.locals, options);
 
       if (globalBinding && usingBinding) {
         reportAmbiguousIdentifier(node, [globalBinding, usingBinding]);
@@ -2146,20 +2150,25 @@ export function createChecker(program: Program): Checker {
   function resolveTypeReferenceSym(
     node: TypeReferenceNode | MemberExpressionNode | IdentifierNode,
     mapper: TypeMapper | undefined,
-    resolveDecorator = false
+    options?: Partial<SymbolResolutionOptions> | boolean
   ): Sym | undefined {
     if (mapper === undefined && referenceSymCache.has(node)) {
       return referenceSymCache.get(node);
     }
-    const sym = resolveTypeReferenceSymInternal(node, mapper, resolveDecorator);
+
+    const resolvedOptions: SymbolResolutionOptions =
+      typeof options === "boolean"
+        ? { ...defaultSymbolResolutionOptions, resolveDecorators: options }
+        : { ...defaultSymbolResolutionOptions, ...(options ?? {}) };
+    const sym = resolveTypeReferenceSymInternal(node, mapper, resolvedOptions);
     referenceSymCache.set(node, sym);
     return sym;
   }
 
   function resolveTypeReferenceSymInternal(
     node: TypeReferenceNode | MemberExpressionNode | IdentifierNode,
     mapper: TypeMapper | undefined,
-    resolveDecorator = false
+    options: SymbolResolutionOptions
   ): Sym | undefined {
     if (hasParseError(node)) {
       // Don't report synthetic identifiers used for parser error recovery.
@@ -2168,7 +2177,7 @@ export function createChecker(program: Program): Checker {
     }
 
     if (node.kind === SyntaxKind.TypeReference) {
-      return resolveTypeReferenceSym(node.target, mapper, resolveDecorator);
+      return resolveTypeReferenceSym(node.target, mapper, options);
     }
 
     if (node.kind === SyntaxKind.MemberExpression) {
@@ -2179,21 +2188,21 @@ export function createChecker(program: Program): Checker {
 
       // when resolving a type reference based on an alias, unwrap the alias.
       if (base.flags & SymbolFlags.Alias) {
-        base = getAliasedSymbol(base, mapper);
+        base = getAliasedSymbol(base, mapper, options);
         if (!base) {
           return undefined;
         }
       }
 
       if (node.selector === ".") {
-        return resolveMemberInContainer(node, base, mapper, resolveDecorator);
+        return resolveMemberInContainer(node, base, mapper, options);
       } else {
         return resolveMetaProperty(node, base);
       }
     }
 
     if (node.kind === SyntaxKind.Identifier) {
-      const sym = resolveIdentifierInScope(node, mapper, resolveDecorator);
+      const sym = resolveIdentifierInScope(node, mapper, options);
       if (!sym) return undefined;
 
       return sym.flags & SymbolFlags.Using ? sym.symbolSource : sym;
@@ -2206,10 +2215,10 @@ export function createChecker(program: Program): Checker {
     node: MemberExpressionNode,
     base: Sym,
     mapper: TypeMapper | undefined,
-    resolveDecorator: boolean
+    options: SymbolResolutionOptions
   ) {
     if (base.flags & SymbolFlags.Namespace) {
-      const symbol = resolveIdentifierInTable(node.id, base.exports, resolveDecorator);
+      const symbol = resolveIdentifierInTable(node.id, base.exports, options);
       if (!symbol) {
         reportCheckerDiagnostic(
           createDiagnostic({
@@ -2247,7 +2256,7 @@ export function createChecker(program: Program): Checker {
 
       return undefined;
     } else if (base.flags & SymbolFlags.MemberContainer) {
-      if (isTemplatedNode(base.declarations[0])) {
+      if (options.checkTemplateTypes && isTemplatedNode(base.declarations[0])) {
         const type =
           base.flags & SymbolFlags.LateBound
             ? base.type!
@@ -2256,7 +2265,7 @@ export function createChecker(program: Program): Checker {
           lateBindMembers(type, base);
         }
       }
-      const sym = resolveIdentifierInTable(node.id, base.members!, resolveDecorator);
+      const sym = resolveIdentifierInTable(node.id, base.members!, options);
       if (!sym) {
         reportCheckerDiagnostic(
           createDiagnostic({
@@ -2287,7 +2296,10 @@ export function createChecker(program: Program): Checker {
   }
 
   function resolveMetaProperty(node: MemberExpressionNode, base: Sym) {
-    const resolved = resolveIdentifierInTable(node.id, base.metatypeMembers);
+    const resolved = resolveIdentifierInTable(node.id, base.metatypeMembers, {
+      resolveDecorators: false,
+      checkTemplateTypes: false,
+    });
     if (!resolved) {
       reportCheckerDiagnostic(
         createDiagnostic({
@@ -2326,8 +2338,23 @@ export function createChecker(program: Program): Checker {
    * (i.e. they contain symbols we don't know until we've instantiated the type and the type is an
    * instantiation) we late bind the container which creates the symbol that will hold its members.
    */
-  function getAliasedSymbol(aliasSymbol: Sym, mapper: TypeMapper | undefined): Sym | undefined {
-    const aliasType = getTypeForNode(aliasSymbol.declarations[0] as AliasStatementNode, mapper);
+  function getAliasedSymbol(
+    aliasSymbol: Sym,
+    mapper: TypeMapper | undefined,
+    options: SymbolResolutionOptions
+  ): Sym | undefined {
+    const node = aliasSymbol.declarations[0];
+    const targetNode = node.kind === SyntaxKind.AliasStatement ? node.value : node;
+    const sym = resolveTypeReferenceSymInternal(targetNode as any, mapper, options);
+    if (sym === undefined) {
+      return undefined;
+    }
+    const resolvedTargetNode = sym.declarations[0];
+    if (!options.checkTemplateTypes || !isTemplatedNode(resolvedTargetNode)) {
+      return sym;
+    }
+
+    const aliasType = getTypeForNode(node as AliasStatementNode, mapper);
     if (isErrorType(aliasType)) {
       return undefined;
     }
@@ -2827,7 +2854,9 @@ export function createChecker(program: Program): Checker {
             table.set("parameters", node.signature.parameters.symbol);
             table.set("returnType", node.signature.returnType.symbol);
           } else {
-            const sig = resolveTypeReferenceSym(node.signature.baseOperation, undefined);
+            const sig = resolveTypeReferenceSym(node.signature.baseOperation, undefined, {
+              checkTemplateTypes: false,
+            });
             if (sig) {
               visit(sig.declarations[0], sig);
               const sigTable = getOrCreateAugmentedSymbolTable(sig.metatypeMembers!);
@@ -6076,3 +6105,22 @@ enum Related {
   true = 1,
   maybe = 2,
 }
+
+interface SymbolResolutionOptions {
+  /**
+   * Should resolving the symbol lookup for decorators as well.
+   * @default false
+   */
+  resolveDecorators: boolean;
+
+  /**
+   * Should the symbol resolution instantiate templates and do a late bind of symbols.
+   * @default true
+   */
+  checkTemplateTypes: boolean;
+}
+
+const defaultSymbolResolutionOptions: SymbolResolutionOptions = {
+  resolveDecorators: false,
+  checkTemplateTypes: true,
+};

packages/compiler/test/checker/augment-decorators.test.ts

@@ -68,6 +68,7 @@ describe("compiler: checker: augment decorators", () => {
         },
       });
     });
+
     it("can be defined at the root of document", async () => {
       testHost.addTypeSpecFile(
         "test.tsp",
@@ -119,6 +120,36 @@ describe("compiler: checker: augment decorators", () => {
       const { Foo } = await testHost.compile("test.tsp");
       strictEqual(Foo, blueThing);
     });
+
+    // Regression for https://github.com/microsoft/typespec/issues/2600
+    it("alias of instantiated template doesn't interfere with augment decorators", async () => {
+      // Here we could have add an issue where `Foo` would have been checked before the `@@blue` augment decorator could be run
+      // As we resolve the member symbols and meta types early,
+      // alias `FactoryString` would have checked the template instance `Factory<string>`
+      // which would then have checked `Foo` and then `@@blue` wouldn't have been run
+      testHost.addTypeSpecFile(
+        "test.tsp",
+        `
+        import "./test.js";
+
+        @test model Foo {};
+
+        interface Factory<T> {
+          op Action(): Foo;
+        }
+
+        alias FactoryString = Factory<string>;
+
+        op test is FactoryString.Action;
+
+        @@doc(Foo, "This doc");
+        @@blue(Foo);
+        `
+      );
+
+      const { Foo } = await testHost.compile("test.tsp");
+      strictEqual(Foo, blueThing);
+    });
   });
 
   describe("augment types", () => {