[core] Disallow overriding a required property with an optional prope…

…rty (#3659)

Closes #3539 

This change disallows overriding a required inherited property with an
optional property.

This error does not take precedence over regular property assignability
rules. In case the property _both_ is optional when the overridden
property is required _and_ has a non-assignable type, both errors will
be reported.

---------

Co-authored-by: Will Temple <[email protected]>

.chronus/changes/witemple-no-override-required-as-optional-2024-5-25-14-39-2.md

@@ -0,0 +1,23 @@
+---
+changeKind: breaking
+packages:
+  - "@typespec/compiler"
+---
+
+Disallows overriding a required inherited property with an optional property.
+
+In previous versions of TypeSpec, it was possible to override a required property with an optional property. This is no longer allowed. This change may result in errors in your code if you were relying on this bug, but specifications that used this behavior are likely to have been exposed to errors resulting from incoherent type checking behavior.
+
+The following example demonstrates the behavior that is no longer allowed:
+
+```tsp
+model Base {
+  example: string;
+}
+
+model Child extends Base {
+  example?: string;
+}
+```
+
+In this example, the `Child` model overrides the `example` property from the `Base` model with an optional property. This is no longer allowed.

packages/compiler/src/core/checker.ts

@@ -4618,6 +4618,8 @@ export function createChecker(program: Program): Checker {
       const parentType = getTypeName(overriddenProp.type);
       const newPropType = getTypeName(newProp.type);
 
+      let invalid = false;
+
       if (!isAssignable) {
         reportCheckerDiagnostic(
           createDiagnostic({
@@ -4626,8 +4628,22 @@ export function createChecker(program: Program): Checker {
             target: diagnosticTarget ?? newProp,
           })
         );
-        return;
+        invalid = true;
       }
+
+      if (!overriddenProp.optional && newProp.optional) {
+        reportCheckerDiagnostic(
+          createDiagnostic({
+            code: "override-property-mismatch",
+            messageId: "disallowedOptionalOverride",
+            format: { propName: overriddenProp.name },
+            target: diagnosticTarget ?? newProp,
+          })
+        );
+        invalid = true;
+      }
+
+      if (invalid) return;
     }
 
     properties.set(newProp.name, newProp);

packages/compiler/src/core/messages.ts

@@ -357,6 +357,7 @@ const diagnostics = {
     severity: "error",
     messages: {
       default: paramMessage`Model has an inherited property named ${"propName"} of type ${"propType"} which cannot override type ${"parentType"}`,
+      disallowedOptionalOverride: paramMessage`Model has a required inherited property named ${"propName"} which cannot be overridden as optional`,
     },
   },
   "extend-scalar": {

packages/compiler/test/checker/model.test.ts

@@ -445,6 +445,73 @@ describe("compiler: models", () => {
       ]);
     });
 
+    it("disallows subtype overriding required parent property with optional property", async () => {
+      testHost.addTypeSpecFile(
+        "main.tsp",
+        `
+        model A { x: int32; }
+        model B extends A { x?: int32; }
+        `
+      );
+
+      const diagnostics = await testHost.diagnose("main.tsp");
+      expectDiagnostics(diagnostics, [
+        {
+          code: "override-property-mismatch",
+          severity: "error",
+          message:
+            "Model has a required inherited property named x which cannot be overridden as optional",
+        },
+      ]);
+    });
+
+    it("disallows subtype overriding required parent property with optional through multiple levels of inheritance", async () => {
+      testHost.addTypeSpecFile(
+        "main.tsp",
+        `
+        model A { x: int32; }
+        model B extends A { }
+        model C extends B { x?: int16; }
+        `
+      );
+
+      const diagnostics = await testHost.diagnose("main.tsp");
+      expectDiagnostics(diagnostics, [
+        {
+          code: "override-property-mismatch",
+          severity: "error",
+          message:
+            "Model has a required inherited property named x which cannot be overridden as optional",
+        },
+      ]);
+    });
+
+    it("shows both errors when an override is optional and not assignable", async () => {
+      testHost.addTypeSpecFile(
+        "main.tsp",
+        `
+        model A { x: int32; }
+        model B extends A { x?: string; }
+        `
+      );
+
+      const diagnostics = await testHost.diagnose("main.tsp");
+      expectDiagnostics(diagnostics, [
+        {
+          code: "override-property-mismatch",
+          severity: "error",
+          message:
+            "Model has an inherited property named x of type string which cannot override type int32",
+        },
+        {
+          code: "override-property-mismatch",
+          severity: "error",
+          message:
+            "Model has a required inherited property named x which cannot be overridden as optional",
+        },
+      ]);
+    });
+
     it("allow multiple overrides", async () => {
       testHost.addTypeSpecFile(
         "main.tsp",