Add useDefineForClassFields flag for Set -> Define property declaration (#33509)

* Disallow property/accessor overrides

Unless the base property or accessor is abstract

* Disallow uninitialised property overrides

This causes quite a few test breaks. We'll probably want to revert many
of them by switching to the upcoming `declare x: number` syntax.

* Updates from design review + fix ancient bug

1. Don't error when overriding properties from interfaces.
2. Fix error when overriding methods with other things. This had no
tests so I assume that the code was always dead and never worked.

* Need to add a couple of errors and squash one

Will update after checking out other branch for a minute

* Everything works so far

Need to test properties initialised in constructor

* Check for constructor initialisation

* change error wording

* Improve error wording

* Add codefix to add missing 'declare'

* Always emit accessors in .d.ts files

* Allow 'declare' on any uninitialised property decl

* Undo code moves

* Let sleeping dogs lie

* Correctly set NodeFlags.Ambient

And simplify redundant parts of check.

* Remove more unneeded code

* Update baselines

* Update baselines

* Update baselines

* Ignore this-property assignments

* Fix base-in-interface check

* Do not error when base parent is interface

* Fix base interface check

* Add missed baselines

* Fix check

* Fix new errors in services

* Fix new errors in services

* Fix errors in testRunner

* Add flag and turn off errors when on

* Structure of new emit is correct, fake content

It is 'hi'.

* Basically right emit

* Fix one last unitialised property declaration

* Haha no I missed another one

* Fix whitespace back to CRLF

* Minor fix and code cleanup

* New test case

* Fix bug in isInitializedProperty

* Updates from design meeting.

1. Change flag name to useDefineForClassFields (and flip polarity).
2. Forbid ES3 + useDefineForClassFields (since there is no
defineProperty).
3. Forbid overriding an abstract property-with-initializer with an
accessor.

* Update baselines

* Object.defineProperty for methods too

Using code from Ron from his upcoming refactor of the factory functions.

* Update slow baselines

* Improve error message

* Update src/compiler/transformers/utilities.ts

Co-Authored-By: Andrew Branch <[email protected]>

* Add test of computed properties

* Remove done TODO

Author: sandersn

src/compiler/checker.ts

@@ -29904,7 +29904,6 @@ namespace ts {
         }
 
         function checkKindsOfPropertyMemberOverrides(type: InterfaceType, baseType: BaseType): void {
-
             // TypeScript 1.0 spec (April 2014): 8.2.3
             // A derived class inherits all members from its base class it doesn't override.
             // Inheritance means that a derived class implicitly contains all non - overridden members of the base class.
@@ -29938,7 +29937,6 @@ namespace ts {
                 // type declaration, derived and base resolve to the same symbol even in the case of generic classes.
                 if (derived === base) {
                     // derived class inherits base without override/redeclaration
-
                     const derivedClassDecl = getClassLikeDeclarationOfSymbol(type.symbol)!;
 
                     // It is an error to inherit an abstract member without implementing it or being declared abstract.
@@ -29975,14 +29973,53 @@ namespace ts {
                         continue;
                     }
 
-                    if (isPrototypeProperty(base) || base.flags & SymbolFlags.PropertyOrAccessor && derived.flags & SymbolFlags.PropertyOrAccessor) {
-                        // method is overridden with method or property/accessor is overridden with property/accessor - correct case
-                        continue;
-                    }
-
                     let errorMessage: DiagnosticMessage;
-                    if (isPrototypeProperty(base)) {
-                        if (derived.flags & SymbolFlags.Accessor) {
+                    const basePropertyFlags = base.flags & SymbolFlags.PropertyOrAccessor;
+                    const derivedPropertyFlags = derived.flags & SymbolFlags.PropertyOrAccessor;
+                    if (basePropertyFlags && derivedPropertyFlags) {
+                        // property/accessor is overridden with property/accessor
+                        if (!compilerOptions.useDefineForClassFields
+                            || baseDeclarationFlags & ModifierFlags.Abstract && !(base.valueDeclaration && isPropertyDeclaration(base.valueDeclaration) && base.valueDeclaration.initializer)
+                            || base.valueDeclaration && base.valueDeclaration.parent.kind === SyntaxKind.InterfaceDeclaration
+                            || derived.valueDeclaration && isBinaryExpression(derived.valueDeclaration)) {
+                            // when the base property is abstract or from an interface, base/derived flags don't need to match
+                            // same when the derived property is from an assignment
+                            continue;
+                        }
+                        if (basePropertyFlags !== SymbolFlags.Property && derivedPropertyFlags === SymbolFlags.Property) {
+                            errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_property;
+                        }
+                        else if (basePropertyFlags === SymbolFlags.Property && derivedPropertyFlags !== SymbolFlags.Property) {
+                            errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_accessor;
+                        }
+                        else {
+                            const uninitialized = find(derived.declarations, d => d.kind === SyntaxKind.PropertyDeclaration && !(d as PropertyDeclaration).initializer);
+                            if (uninitialized
+                                && !(derived.flags & SymbolFlags.Transient)
+                                && !(baseDeclarationFlags & ModifierFlags.Abstract)
+                                && !(derivedDeclarationFlags & ModifierFlags.Abstract)
+                                && !derived.declarations.some(d => d.flags & NodeFlags.Ambient)) {
+                                const constructor = findConstructorDeclaration(getClassLikeDeclarationOfSymbol(type.symbol)!);
+                                const propName = (uninitialized as PropertyDeclaration).name;
+                                if ((uninitialized as PropertyDeclaration).exclamationToken
+                                    || !constructor
+                                    || !isIdentifier(propName)
+                                    || !strictNullChecks
+                                    || !isPropertyInitializedInConstructor(propName, type, constructor)) {
+                                    const errorMessage = Diagnostics.Property_0_will_overwrite_the_base_property_in_1_If_this_is_intentional_add_an_initializer_Otherwise_add_a_declare_modifier_or_remove_the_redundant_declaration;
+                                    error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, symbolToString(base), typeToString(baseType));
+                                }
+                            }
+                            // correct case
+                            continue;
+                        }
+                    }
+                    else if (isPrototypeProperty(base)) {
+                        if (isPrototypeProperty(derived)) {
+                            // method is overridden with method -- correct case
+                            continue;
+                        }
+                        else if (derived.flags & SymbolFlags.Accessor) {
                             errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_accessor;
                         }
                         else {
@@ -30044,6 +30081,9 @@ namespace ts {
             }
             const constructor = findConstructorDeclaration(node);
             for (const member of node.members) {
+                if (getModifierFlags(member) & ModifierFlags.Ambient) {
+                    continue;
+                }
                 if (isInstancePropertyWithoutInitializer(member)) {
                     const propName = (<PropertyDeclaration>member).name;
                     if (isIdentifier(propName)) {
@@ -32970,7 +33010,7 @@ namespace ts {
                         else if (flags & ModifierFlags.Async) {
                             return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_in_an_ambient_context, "async");
                         }
-                        else if (isClassLike(node.parent)) {
+                        else if (isClassLike(node.parent) && !isPropertyDeclaration(node)) {
                             return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_appear_on_a_class_element, "declare");
                         }
                         else if (node.kind === SyntaxKind.Parameter) {

src/compiler/commandLineParser.ts

@@ -873,6 +873,13 @@ namespace ts {
             category: Diagnostics.Advanced_Options,
             description: Diagnostics.Disable_strict_checking_of_generic_signatures_in_function_types,
         },
+        {
+            name: "useDefineForClassFields",
+            type: "boolean",
+            affectsSemanticDiagnostics: true,
+            category: Diagnostics.Advanced_Options,
+            description: Diagnostics.Emit_class_fields_with_Define_instead_of_Set,
+        },
         {
             name: "keyofStringsOnly",
             type: "boolean",

src/compiler/diagnosticMessages.json

@@ -2233,6 +2233,19 @@
         "category": "Error",
         "code": 2609
     },
+    "Class '{0}' defines instance member accessor '{1}', but extended class '{2}' defines it as instance member property.": {
+        "category": "Error",
+        "code": 2610
+    },
+    "Class '{0}' defines instance member property '{1}', but extended class '{2}' defines it as instance member accessor.": {
+        "category": "Error",
+        "code": 2611
+    },
+    "Property '{0}' will overwrite the base property in '{1}'. If this is intentional, add an initializer. Otherwise, add a 'declare' modifier or remove the redundant declaration.": {
+        "category": "Error",
+        "code": 2612
+    },
+
     "Cannot augment module '{0}' with value exports because it resolves to a non-module entity.": {
         "category": "Error",
         "code": 2649
@@ -3112,6 +3125,10 @@
         "category": "Error",
         "code": 5047
     },
+    "Option '{0}' cannot be specified when option 'target' is 'ES3'.": {
+        "category": "Error",
+        "code": 5048
+    },
     "Option '{0} can only be used when either option '--inlineSourceMap' or option '--sourceMap' is provided.": {
         "category": "Error",
         "code": 5051
@@ -3966,6 +3983,10 @@
     "Conflicts are in this file.": {
         "category": "Message",
         "code": 6201
+    },
+        "Project references may not form a circular graph. Cycle detected: {0}": {
+        "category": "Error",
+        "code": 6202
     },
     "'{0}' was also declared here.": {
         "category": "Message",
@@ -4043,6 +4064,10 @@
         "category": "Message",
         "code": 6221
     },
+    "Emit class fields with Define instead of Set.": {
+        "category": "Message",
+        "code": 6222
+    },
 
     "Projects to reference": {
         "category": "Message",
@@ -4052,10 +4077,7 @@
         "category": "Message",
         "code": 6302
     },
-    "Project references may not form a circular graph. Cycle detected: {0}": {
-        "category": "Error",
-        "code": 6202
-    },
+
     "Composite projects may not disable declaration emit.": {
         "category": "Error",
         "code": 6304
@@ -5180,6 +5202,14 @@
         "category": "Message",
         "code": 95093
     },
+    "Prefix with 'declare'": {
+        "category": "Message",
+        "code": 95094
+    },
+    "Prefix all incorrect property declarations with 'declare'": {
+        "category": "Message",
+        "code": 95095
+    },
 
     "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
         "category": "Error",

src/compiler/factory.ts

@@ -484,6 +484,47 @@ namespace ts {
         return node;
     }
 
+    function createMethodCall(object: Expression, methodName: string | Identifier, argumentsList: readonly Expression[]) {
+        return createCall(
+            createPropertyAccess(object, asName(methodName)),
+            /*typeArguments*/ undefined,
+            argumentsList
+        );
+    }
+
+    function createGlobalMethodCall(globalObjectName: string, methodName: string, argumentsList: readonly Expression[]) {
+        return createMethodCall(createIdentifier(globalObjectName), methodName, argumentsList);
+    }
+
+    /* @internal */
+    export function createObjectDefinePropertyCall(target: Expression, propertyName: string | Expression, attributes: Expression) {
+        return createGlobalMethodCall("Object", "defineProperty", [target, asExpression(propertyName), attributes]);
+    }
+
+    function tryAddPropertyAssignment(properties: Push<PropertyAssignment>, propertyName: string, expression: Expression | undefined) {
+        if (expression) {
+            properties.push(createPropertyAssignment(propertyName, expression));
+            return true;
+        }
+        return false;
+    }
+
+    /* @internal */
+    export function createPropertyDescriptor(attributes: PropertyDescriptorAttributes, singleLine?: boolean) {
+        const properties: PropertyAssignment[] = [];
+        tryAddPropertyAssignment(properties, "enumerable", asExpression(attributes.enumerable));
+        tryAddPropertyAssignment(properties, "configurable", asExpression(attributes.configurable));
+
+        let isData = tryAddPropertyAssignment(properties, "writable", asExpression(attributes.writable));
+        isData = tryAddPropertyAssignment(properties, "value", attributes.value) || isData;
+
+        let isAccessor = tryAddPropertyAssignment(properties, "get", attributes.get);
+        isAccessor = tryAddPropertyAssignment(properties, "set", attributes.set) || isAccessor;
+
+        Debug.assert(!(isData && isAccessor), "A PropertyDescriptor may not be both an accessor descriptor and a data descriptor.");
+        return createObjectLiteral(properties, !singleLine);
+    }
+
     export function updateMethod(
         node: MethodDeclaration,
         decorators: readonly Decorator[] | undefined,
@@ -3146,8 +3187,11 @@ namespace ts {
         return isString(name) ? createIdentifier(name) : name;
     }
 
-    function asExpression(value: string | number | Expression) {
-        return isString(value) || typeof value === "number" ? createLiteral(value) : value;
+    function asExpression<T extends Expression | undefined>(value: string | number | boolean | T): T | StringLiteral | NumericLiteral | BooleanLiteral {
+        return typeof value === "string" ? createStringLiteral(value) :
+            typeof value === "number" ? createNumericLiteral(""+value) :
+            typeof value === "boolean" ? value ? createTrue() : createFalse() :
+        value;
     }
 
     function asNodeArray<T extends Node>(array: readonly T[]): NodeArray<T>;

src/compiler/parser.ts

@@ -5995,8 +5995,16 @@ namespace ts {
                 token() === SyntaxKind.NumericLiteral ||
                 token() === SyntaxKind.AsteriskToken ||
                 token() === SyntaxKind.OpenBracketToken) {
-
-                return parsePropertyOrMethodDeclaration(<PropertyDeclaration | MethodDeclaration>node);
+                const isAmbient = node.modifiers && some(node.modifiers, isDeclareModifier);
+                if (isAmbient) {
+                    for (const m of node.modifiers!) {
+                        m.flags |= NodeFlags.Ambient;
+                    }
+                    return doInsideOfContext(NodeFlags.Ambient, () => parsePropertyOrMethodDeclaration(node as PropertyDeclaration | MethodDeclaration));
+                }
+                else {
+                    return parsePropertyOrMethodDeclaration(node as PropertyDeclaration | MethodDeclaration);
+                }
             }
 
             if (node.decorators || node.modifiers) {

src/compiler/program.ts

@@ -3073,6 +3073,10 @@ namespace ts {
                 }
             }
 
+            if (options.useDefineForClassFields && languageVersion === ScriptTarget.ES3) {
+                createDiagnosticForOptionName(Diagnostics.Option_0_cannot_be_specified_when_option_target_is_ES3, "useDefineForClassFields");
+            }
+
             if (!options.noEmit && options.allowJs && getEmitDeclarations(options)) {
                 createDiagnosticForOptionName(Diagnostics.Option_0_cannot_be_specified_with_option_1, "allowJs", getEmitDeclarationOptionName(options));
             }