Make most Node properties read-only

microsoft · rbuckton · Jun 16, 2020 · Nov 21, 2019 · Nov 23, 2019 · Dec 9, 2019
commit 602efc5b87936969ff56cc4dd54756d61eb1fb0c
diff --git a/src/compat/deprecations.ts b/src/compat/deprecations.ts
@@ -1928,7 +1928,11 @@ namespace ts {
     });
 
     /**
-     * Creates a shallow, memberwise clone of a node for mutation.
+     * Creates a shallow, memberwise clone of a node ~for mutation~ with its `pos`, `end`, and `parent` set.
+     *
+     * NOTE: It is unsafe to change any properties of a `Node` that relate to its AST children, as those changes won't be
+     * captured with respect to transformations.
+     *
      * @deprecated Use `factory.cloneNode` instead and set `pos`, `end`, and `parent` as needed.
      */
     export function getMutableClone<T extends Node>(node: T): T {

diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -584,7 +584,7 @@ namespace ts {
         // All container nodes are kept on a linked list in declaration order. This list is used by
         // the getLocalNameOfContainer function in the type checker to validate that the local name
         // used for a container is unique.
-        function bindContainer(node: Node, containerFlags: ContainerFlags) {
+        function bindContainer(node: Mutable<Node>, containerFlags: ContainerFlags) {
             // Before we recurse into a node's children, we first save the existing parent, container
             // and block-container.  Then after we pop out of processing the children, we restore
             // these saved values.
@@ -1757,7 +1757,7 @@ namespace ts {
             return !!body && body.statements.some(s => isExportDeclaration(s) || isExportAssignment(s));
         }
 
-        function setExportContextFlag(node: ModuleDeclaration | SourceFile) {
+        function setExportContextFlag(node: Mutable<ModuleDeclaration | SourceFile>) {
             // A declaration source file or ambient module declaration that contains no export declarations (but possibly regular
             // declarations with export modifiers) is an export context in which declarations are implicitly exported.
             if (node.flags & NodeFlags.Ambient && !hasExportDeclarations(node)) {

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
diff --git a/src/compiler/core.ts b/src/compiler/core.ts
@@ -719,10 +719,18 @@ namespace ts {
         return [...array1, ...array2];
     }
 
+    function selectIndex(_: unknown, i: number) {
+        return i;
+    }
+
+    export function indicesOf(array: readonly unknown[]): number[] {
+        return array.map(selectIndex);
+    }
+
     function deduplicateRelational<T>(array: readonly T[], equalityComparer: EqualityComparer<T>, comparer: Comparer<T>) {
         // Perform a stable sort of the array. This ensures the first entry in a list of
         // duplicates remains the first entry in the result.
-        const indices = array.map((_, i) => i);
+        const indices = indicesOf(array);
         stableSortIndices(array, indices, comparer);
 
         let last = array[indices[0]];
@@ -1028,7 +1036,7 @@ namespace ts {
      * Stable sort of an array. Elements equal to each other maintain their relative position in the array.
      */
     export function stableSort<T>(array: readonly T[], comparer: Comparer<T>): SortedReadonlyArray<T> {
-        const indices = array.map((_, i) => i);
+        const indices = indicesOf(array);
         stableSortIndices(array, indices, comparer);
         return indices.map(i => array[i]) as SortedArray<T> as SortedReadonlyArray<T>;
     }

diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -685,7 +685,7 @@ namespace ts {
                 return statement;
             });
             const eofToken = factory.createToken(SyntaxKind.EndOfFileToken);
-            const sourceFile = factory.createSourceFile(statements ?? [], eofToken);
+            const sourceFile = factory.createSourceFile(statements ?? [], eofToken, NodeFlags.None);
             sourceFile.fileName = getRelativePathFromDirectory(
                 host.getCurrentDirectory(),
                 getNormalizedAbsolutePath(fileName, buildInfoDirectory),

diff --git a/src/compiler/factory/nodeFactory.ts b/src/compiler/factory/nodeFactory.ts
diff --git a/src/compiler/factory/utilities.ts b/src/compiler/factory/utilities.ts
@@ -37,7 +37,7 @@ namespace ts {
     function createJsxFactoryExpressionFromEntityName(factory: NodeFactory, jsxFactory: EntityName, parent: JsxOpeningLikeElement | JsxOpeningFragment): Expression {
         if (isQualifiedName(jsxFactory)) {
             const left = createJsxFactoryExpressionFromEntityName(factory, jsxFactory.left, parent);
-            const right = factory.createIdentifier(idText(jsxFactory.right));
+            const right = factory.createIdentifier(idText(jsxFactory.right)) as Mutable<Identifier>;
             right.escapedText = jsxFactory.right.escapedText;
             return factory.createPropertyAccess(left, right);
         }
@@ -780,4 +780,31 @@ namespace ts {
                 return <readonly BindingOrAssignmentElement[]>name.properties;
         }
     }
+
+    export function canHaveModifiers(node: Node): node is HasModifiers {
+        const kind = node.kind;
+        return kind === SyntaxKind.Parameter
+            || kind === SyntaxKind.PropertySignature
+            || kind === SyntaxKind.PropertyDeclaration
+            || kind === SyntaxKind.MethodSignature
+            || kind === SyntaxKind.MethodDeclaration
+            || kind === SyntaxKind.Constructor
+            || kind === SyntaxKind.GetAccessor
+            || kind === SyntaxKind.SetAccessor
+            || kind === SyntaxKind.IndexSignature
+            || kind === SyntaxKind.FunctionExpression
+            || kind === SyntaxKind.ArrowFunction
+            || kind === SyntaxKind.ClassExpression
+            || kind === SyntaxKind.VariableStatement
+            || kind === SyntaxKind.FunctionDeclaration
+            || kind === SyntaxKind.ClassDeclaration
+            || kind === SyntaxKind.InterfaceDeclaration
+            || kind === SyntaxKind.TypeAliasDeclaration
+            || kind === SyntaxKind.EnumDeclaration
+            || kind === SyntaxKind.ModuleDeclaration
+            || kind === SyntaxKind.ImportEqualsDeclaration
+            || kind === SyntaxKind.ImportDeclaration
+            || kind === SyntaxKind.ExportAssignment
+            || kind === SyntaxKind.ExportDeclaration;
+    }
 }
diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts
@@ -568,7 +568,7 @@ namespace ts {
         const newSourceFile = IncrementalParser.updateSourceFile(sourceFile, newText, textChangeRange, aggressiveChecks);
         // Because new source file node is created, it may not have the flag PossiblyContainDynamicImport. This is the case if there is no new edit to add dynamic import.
         // We will manually port the flag to the new source file.
-        newSourceFile.flags |= (sourceFile.flags & NodeFlags.PermanentlySetIncrementalFlags);
+        (newSourceFile as Mutable<SourceFile>).flags |= (sourceFile.flags & NodeFlags.PermanentlySetIncrementalFlags);
         return newSourceFile;
     }
 
@@ -824,8 +824,7 @@ namespace ts {
             }
 
             // Set source file so that errors will be reported with this file name
-            const sourceFile = createSourceFile(fileName, ScriptTarget.ES2015, ScriptKind.JSON, /*isDeclaration*/ false, statements, endOfFileToken);
-            sourceFile.flags |= sourceFlags;
+            const sourceFile = createSourceFile(fileName, ScriptTarget.ES2015, ScriptKind.JSON, /*isDeclaration*/ false, statements, endOfFileToken, sourceFlags);
 
             if (setParentNodes) {
                 fixupParentReferences(sourceFile);
@@ -923,8 +922,7 @@ namespace ts {
             Debug.assert(token() === SyntaxKind.EndOfFileToken);
             const endOfFileToken = addJSDocComment(parseTokenNode<EndOfFileToken>());
 
-            const sourceFile = createSourceFile(fileName, languageVersion, scriptKind, isDeclarationFile, statements, endOfFileToken);
-            sourceFile.flags |= sourceFlags;
+            const sourceFile = createSourceFile(fileName, languageVersion, scriptKind, isDeclarationFile, statements, endOfFileToken, sourceFlags);
 
             // A member of ReadonlyArray<T> isn't assignable to a member of T[] (and prevents a direct cast) - but this is where we set up those members so they can be readonly in the future
             processCommentPragmas(sourceFile as {} as PragmaContext, sourceText);
@@ -994,10 +992,10 @@ namespace ts {
             }
         }
 
-        function createSourceFile(fileName: string, languageVersion: ScriptTarget, scriptKind: ScriptKind, isDeclarationFile: boolean, statements: readonly Statement[], endOfFileToken: EndOfFileToken): SourceFile {
+        function createSourceFile(fileName: string, languageVersion: ScriptTarget, scriptKind: ScriptKind, isDeclarationFile: boolean, statements: readonly Statement[], endOfFileToken: EndOfFileToken, flags: NodeFlags): SourceFile {
             // code from createNode is inlined here so createNode won't have to deal with special case of creating source files
             // this is quite rare comparing to other nodes and createNode should be as fast as possible
-            const sourceFile = factory.createSourceFile(statements, endOfFileToken);
+            const sourceFile = factory.createSourceFile(statements, endOfFileToken, flags);
             sourceFile.pos = 0;
             sourceFile.end = sourceText.length;
             sourceFile.text = sourceText;
@@ -1411,15 +1409,15 @@ namespace ts {
             node.end = end === undefined ? scanner.getStartPos() : end;
 
             if (contextFlags) {
-                node.flags |= contextFlags;
+                (node as Mutable<T>).flags |= contextFlags;
             }
 
             // Keep track on the node if we encountered an error while parsing it.  If we did, then
             // we cannot reuse the node incrementally.  Once we've marked this node, clear out the
             // flag so that we don't mark any subsequent nodes.
             if (parseErrorBeforeNextFinishedNode) {
                 parseErrorBeforeNextFinishedNode = false;
-                node.flags |= NodeFlags.ThisNodeHasError;
+                (node as Mutable<T>).flags |= NodeFlags.ThisNodeHasError;
             }
 
             return node;
@@ -3040,7 +3038,7 @@ namespace ts {
                 const node = factory.createOptionalTypeNode(type.type);
                 node.pos = type.pos;
                 node.end = type.end;
-                node.flags = type.flags;
+                (node as Mutable<Node>).flags = type.flags;
                 return node;
             }
             return type;
@@ -4783,7 +4781,7 @@ namespace ts {
                 parseTemplateExpression()
             );
             if (questionDotToken || tag.flags & NodeFlags.OptionalChain) {
-                tagExpression.flags |= NodeFlags.OptionalChain;
+                (tagExpression as Mutable<Node>).flags |= NodeFlags.OptionalChain;
             }
             factory.trackExtraneousChildNode(tagExpression, tagExpression.questionDotToken = questionDotToken);
             return finishNode(tagExpression, pos);
@@ -5019,7 +5017,7 @@ namespace ts {
             // CoverInitializedName[Yield] :
             //     IdentifierReference[?Yield] Initializer[In, ?Yield]
             // this is necessary because ObjectLiteral productions are also used to cover grammar for ObjectAssignmentPattern
-            let node: ShorthandPropertyAssignment | PropertyAssignment;
+            let node: Mutable<ShorthandPropertyAssignment | PropertyAssignment>;
             const isShorthandPropertyAssignment = tokenIsIdentifier && (token() !== SyntaxKind.ColonToken);
             if (isShorthandPropertyAssignment) {
                 const equalsToken = parseOptionalToken(SyntaxKind.EqualsToken);
@@ -5323,13 +5321,15 @@ namespace ts {
             // ThrowStatement[Yield] :
             //      throw [no LineTerminator here]Expression[In, ?Yield];
 
+            const pos = getNodePos();
+            parseExpected(SyntaxKind.ThrowKeyword);
+
             // Because of automatic semicolon insertion, we need to report error if this
             // throw could be terminated with a semicolon.  Note: we can't call 'parseExpression'
             // directly as that might consume an expression on the following line.
             // We just return 'undefined' in that case.  The actual error will be reported in the
             // grammar walker.
-            const pos = getNodePos();
-            parseExpected(SyntaxKind.ThrowKeyword);
+            // TODO(rbuckton): Should we use `createMissingNode` here instead?
             const expression = scanner.hasPrecedingLineBreak() ? undefined : allowInAnd(parseExpression);
             parseSemicolon();
             return finishNode(factory.createThrow(expression!), pos);
@@ -5667,7 +5667,7 @@ namespace ts {
             const modifiers = parseModifiers();
             if (isAmbient) {
                 for (const m of modifiers!) {
-                    m.flags |= NodeFlags.Ambient;
+                    (m as Mutable<Node>).flags |= NodeFlags.Ambient;
                 }
                 return doInsideOfContext(NodeFlags.Ambient, () => parseDeclarationWorker(pos, hasJSDoc, decorators, modifiers));
             }
@@ -5722,7 +5722,7 @@ namespace ts {
                     if (decorators || modifiers) {
                         // We reached this point because we encountered decorators and/or modifiers and assumed a declaration
                         // would follow. For recovery and error reporting purposes, return an incomplete declaration.
-                        const missing = createMissingNode<Statement>(SyntaxKind.MissingDeclaration, /*reportAtCurrentPosition*/ true, Diagnostics.Declaration_expected);
+                        const missing = createMissingNode<MissingDeclaration>(SyntaxKind.MissingDeclaration, /*reportAtCurrentPosition*/ true, Diagnostics.Declaration_expected);
                         missing.pos = pos;
                         missing.decorators = decorators;
                         missing.modifiers = modifiers;
@@ -6004,7 +6004,7 @@ namespace ts {
                 : factory.createSetAccessorDeclaration(decorators, modifiers, name, parameters, body);
             // Keep track of `typeParameters` (for both) and `type` (for setters) if they were parsed those indicate grammar errors
             if (typeParameters) factory.trackExtraneousChildNodes(node, node.typeParameters = typeParameters);
-            if (type && kind === SyntaxKind.SetAccessor) factory.trackExtraneousChildNode(node, node.type = type);
+            if (type && node.kind === SyntaxKind.SetAccessor) factory.trackExtraneousChildNode(node, node.type = type);
             return withJSDoc(finishNode(node, pos), hasJSDoc);
         }
 
@@ -6182,7 +6182,7 @@ namespace ts {
                 const isAmbient = some(modifiers, isDeclareModifier);
                 if (isAmbient) {
                     for (const m of modifiers!) {
-                        m.flags |= NodeFlags.Ambient;
+                        (m as Mutable<Node>).flags |= NodeFlags.Ambient;
                     }
                     return doInsideOfContext(NodeFlags.Ambient, () => parsePropertyOrMethodDeclaration(pos, hasJSDoc, decorators, modifiers));
                 }
@@ -6690,7 +6690,7 @@ namespace ts {
                 currentToken = scanner.scan();
                 const jsDocTypeExpression = parseJSDocTypeExpression();
 
-                const sourceFile = createSourceFile("file.js", ScriptTarget.Latest, ScriptKind.JS, /*isDeclarationFile*/ false, [], factory.createToken(SyntaxKind.EndOfFileToken));
+                const sourceFile = createSourceFile("file.js", ScriptTarget.Latest, ScriptKind.JS, /*isDeclarationFile*/ false, [], factory.createToken(SyntaxKind.EndOfFileToken), NodeFlags.None);
                 const diagnostics = attachFileToDiagnostics(parseDiagnostics, sourceFile);
                 if (jsDocDiagnostics) {
                     sourceFile.jsDocDiagnostics = attachFileToDiagnostics(jsDocDiagnostics, sourceFile);

diff --git a/src/compiler/transformers/classFields.ts b/src/compiler/transformers/classFields.ts
@@ -285,7 +285,7 @@ namespace ts {
                         factory.createConstructorDeclaration(
                             /*decorators*/ undefined,
                             /*modifiers*/ undefined,
-                            parameters,
+                            parameters ?? [],
                             body
                         ),
                         constructor || node
@@ -423,7 +423,7 @@ namespace ts {
                 ? factory.updateComputedPropertyName(property.name, factory.getGeneratedNameForNode(property.name))
                 : property.name;
 
-            const initializer = property.initializer || emitAssignment ? visitNode(property.initializer, visitor, isExpression) : factory.createVoidZero();
+            const initializer = visitNode(property.initializer, visitor, isExpression) ?? factory.createVoidZero();
             if (emitAssignment) {
                 const memberAccess = createMemberAccessForPropertyName(factory, receiver, propertyName, /*location*/ propertyName);
                 return factory.createAssignment(memberAccess, initializer);

diff --git a/src/compiler/transformers/declarations.ts b/src/compiler/transformers/declarations.ts
@@ -1050,16 +1050,14 @@ namespace ts {
         }
 
         function stripExportModifiers(statement: Statement): Statement {
-            if (isImportEqualsDeclaration(statement) || hasModifier(statement, ModifierFlags.Default)) {
+            if (isImportEqualsDeclaration(statement) || hasModifier(statement, ModifierFlags.Default) || !canHaveModifiers(statement)) {
                 // `export import` statements should remain as-is, as imports are _not_ implicitly exported in an ambient namespace
                 // Likewise, `export default` classes and the like and just be `default`, so we preserve their `export` modifiers, too
                 return statement;
             }
-            // TODO(rbuckton): Does this need to be parented?
-            const clone = setParent(setTextRange(factory.cloneNode(statement), statement), statement.parent);
+
             const modifiers = factory.createModifiersFromModifierFlags(getModifierFlags(statement) & (ModifierFlags.All ^ ModifierFlags.Export));
-            clone.modifiers = modifiers.length ? factory.createNodeArray(modifiers) : undefined;
-            return clone;
+            return factory.updateModifiers(statement, modifiers);
         }
 
         function transformTopLevelDeclaration(input: LateVisibilityPaintedStatement) {
@@ -1126,8 +1124,8 @@ namespace ts {
                     ));
                     if (clean && resolver.isExpandoFunctionDeclaration(input)) {
                         const props = resolver.getPropertiesOfContainerFunction(input);
-                        const fakespace = factory.createModuleDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, clean.name || factory.createIdentifier("_default"), factory.createModuleBlock([]), NodeFlags.Namespace);
-                        fakespace.flags ^= NodeFlags.Synthesized; // unset synthesized so it is usable as an enclosing declaration
+                        // Use parseNodeFactory so it is usable as an enclosing declaration
+                        const fakespace = parseNodeFactory.createModuleDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, clean.name || factory.createIdentifier("_default"), factory.createModuleBlock([]), NodeFlags.Namespace);
                         fakespace.parent = enclosingDeclaration as SourceFile | NamespaceDeclaration;
                         fakespace.locals = createSymbolTable(props);
                         fakespace.symbol = props[0].parent!;

diff --git a/src/compiler/transformers/destructuring.ts b/src/compiler/transformers/destructuring.ts
@@ -467,7 +467,6 @@ namespace ts {
         }
         else if (isStringOrNumericLiteralLike(propertyName)) {
             const argumentExpression = factory.cloneNode(propertyName);
-            argumentExpression.text = argumentExpression.text;
             return flattenContext.context.factory.createElementAccess(value, argumentExpression);
         }
         else {