From e5d94225c35876706aafc80cf76c088e207f47ea Mon Sep 17 00:00:00 2001
From: Timothee Guerin <tiguerin@microsoft.com>
Date: Tue, 27 Feb 2024 11:04:00 -0800
Subject: [PATCH] Do not run decorators that have missing args (#2959)

fix [#3940](https://github.com/Azure/typespec-azure-pr/issues/3940)
---
 ...ecorator-missing-args-2024-1-27-18-6-41.md |  8 +++++++
 packages/compiler/src/core/checker.ts         |  6 +++++
 .../compiler/test/checker/decorators.test.ts  | 22 ++++++++++++++-----
 3 files changed, 31 insertions(+), 5 deletions(-)
 create mode 100644 .chronus/changes/fix-dont-run-decorator-missing-args-2024-1-27-18-6-41.md

diff --git a/.chronus/changes/fix-dont-run-decorator-missing-args-2024-1-27-18-6-41.md b/.chronus/changes/fix-dont-run-decorator-missing-args-2024-1-27-18-6-41.md
new file mode 100644
index 0000000000..3aa3798071
--- /dev/null
+++ b/.chronus/changes/fix-dont-run-decorator-missing-args-2024-1-27-18-6-41.md
@@ -0,0 +1,8 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: fix
+packages:
+  - "@typespec/compiler"
+---
+
+Decorators that have missing arguments will not run. This is inline with passing invalid argument to a decorator that would prevent it from running.
diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts
index 3188f1ebbe..cc6287687d 100644
--- a/packages/compiler/src/core/checker.ts
+++ b/packages/compiler/src/core/checker.ts
@@ -3605,6 +3605,12 @@ export function createChecker(program: Program): Checker {
       : declaration.parameters.length;
 
     if (args.length < minArgs || (maxArgs !== undefined && args.length > maxArgs)) {
+      // In the case we have too little args then this decorator is not applicable.
+      // If there is too many args then we can still run the decorator as long as the args are valid.
+      if (args.length < minArgs) {
+        hasError = true;
+      }
+
       if (maxArgs === undefined) {
         reportCheckerDiagnostic(
           createDiagnostic({
diff --git a/packages/compiler/test/checker/decorators.test.ts b/packages/compiler/test/checker/decorators.test.ts
index 610e8c4778..06964b1333 100644
--- a/packages/compiler/test/checker/decorators.test.ts
+++ b/packages/compiler/test/checker/decorators.test.ts
@@ -103,8 +103,9 @@ describe("compiler: checker: decorators", () => {
 
   describe("usage", () => {
     let runner: BasicTestRunner;
-    let calledArgs: any[];
+    let calledArgs: any[] | undefined;
     beforeEach(() => {
+      calledArgs = undefined;
       testHost.addJsFile("test.js", {
         $testDec: (...args: any[]) => (calledArgs = args),
       });
@@ -115,6 +116,7 @@ describe("compiler: checker: decorators", () => {
     });
 
     function expectDecoratorCalledWith(target: unknown, ...args: unknown[]) {
+      ok(calledArgs, "Decorator was not called.");
       strictEqual(calledArgs.length, 2 + args.length);
       strictEqual(calledArgs[0].program, runner.program);
       strictEqual(calledArgs[1], target);
@@ -123,6 +125,10 @@ describe("compiler: checker: decorators", () => {
       }
     }
 
+    function expectDecoratorNotCalled() {
+      strictEqual(calledArgs, undefined);
+    }
+
     it("calls a decorator with no argument", async () => {
       const { Foo } = await runner.compile(`
         extern dec testDec(target: unknown);
@@ -183,20 +189,22 @@ describe("compiler: checker: decorators", () => {
         code: "invalid-argument-count",
         message: "Expected 2 arguments, but got 1.",
       });
+      expectDecoratorNotCalled();
     });
 
     it("errors if not calling with too many arguments", async () => {
-      const diagnostics = await runner.diagnose(`
-        extern dec testDec(target: unknown, arg1: string, arg2?: string);
+      const [{ Foo }, diagnostics] = await runner.compileAndDiagnose(`
+        extern dec testDec(target: unknown, arg1: valueof string, arg2?: valueof string);
 
         @testDec("one", "two", "three")
-        model Foo {}
+        @test model Foo {}
       `);
 
       expectDiagnostics(diagnostics, {
         code: "invalid-argument-count",
         message: "Expected 1-2 arguments, but got 3.",
       });
+      expectDecoratorCalledWith(Foo, "one", "two");
     });
 
     it("errors if not calling with argument and decorator expect none", async () => {
@@ -225,6 +233,7 @@ describe("compiler: checker: decorators", () => {
         code: "invalid-argument-count",
         message: "Expected at least 1 arguments, but got 0.",
       });
+      expectDecoratorNotCalled();
     });
 
     it("errors if target type is incorrect", async () => {
@@ -239,6 +248,7 @@ describe("compiler: checker: decorators", () => {
         code: "decorator-wrong-target",
         message: "Cannot apply @testDec decorator to Foo since it is not assignable to Union",
       });
+      expectDecoratorNotCalled();
     });
 
     it("errors if argument is not assignable to parameter type", async () => {
@@ -253,6 +263,7 @@ describe("compiler: checker: decorators", () => {
         code: "invalid-argument",
         message: "Argument '123' is not assignable to parameter of type 'string'",
       });
+      expectDecoratorNotCalled();
     });
 
     it("errors if argument is not assignable to rest parameter type", async () => {
@@ -273,6 +284,7 @@ describe("compiler: checker: decorators", () => {
           message: "Argument '456' is not assignable to parameter of type 'string'",
         },
       ]);
+      expectDecoratorNotCalled();
     });
 
     describe("value marshalling", () => {
@@ -284,7 +296,7 @@ describe("compiler: checker: decorators", () => {
           @test
           model Foo {}
         `);
-        return calledArgs[2];
+        return calledArgs![2];
       }
 
       describe("passing a string literal", () => {