Merge branch 'main' into gabritto/jsxchildren

Author: gabritto

.github/copilot-instructions.md

@@ -1,28 +1,27 @@
-# TypeScript Test Writing Guide for Copilot
+# Guide for Copilot
 
 This document provides a concise guide for writing TypeScript fourslash tests and compiler tests, along with build instructions.
 
 ## Build Instructions Summary
 
 ### Setup
 1. Install Node.js (current or LTS)
-2. Install hereby: `npm install -g hereby`
-3. Clone the repository: `git clone --depth=1 https://github.com/microsoft/TypeScript`
-4. Install dependencies: `npm ci`
+2. Clone the repository: `git clone --depth=1 https://github.com/microsoft/TypeScript`
+3. Install dependencies: `npm ci`
 
 ### Common Build Tasks
 ```bash
-hereby local             # Build the compiler into built/local
-hereby clean             # Delete the built compiler  
-hereby tests             # Build the test infrastructure
-hereby runtests          # Run all tests
-hereby runtests-parallel # Run tests in parallel (recommended)
-hereby runtests --runner=fourslash # Run only fourslash tests
-hereby runtests --runner=compiler # Run only compiler tests
-hereby runtests --tests=<testPath> # Run specific test
-hereby baseline-accept   # Accept new test baselines
-hereby lint              # Run eslint
-hereby format            # Run code formatting
+npx hereby local             # Build the compiler into built/local
+npx hereby clean             # Delete the built compiler  
+npx hereby tests             # Build the test infrastructure
+npx hereby runtests          # Run all tests
+npx hereby runtests-parallel # Run tests in parallel (recommended)
+npx hereby runtests --runner=fourslash # Run only fourslash tests
+npx hereby runtests --runner=compiler # Run only compiler tests
+npx hereby runtests --tests=<testPath> # Run specific test
+npx hereby baseline-accept   # Accept new test baselines
+npx hereby lint              # Run eslint
+npx hereby format            # Run code formatting
 ```
 
 ## Fourslash Test Syntax Guide
@@ -248,30 +247,57 @@ const config3: Config = { optional: 42 }; // Should error - missing required
 
 ```bash
 # Run a specific fourslash test
-hereby runtests --tests=tests/cases/fourslash/completionForObjectProperty.ts
+npx hereby runtests --tests=tests/cases/fourslash/completionForObjectProperty.ts
 
 # Run a specific compiler test  
-hereby runtests --tests=tests/cases/compiler/abstractClassUnionInstantiation.ts
+npx hereby runtests --tests=tests/cases/compiler/abstractClassUnionInstantiation.ts
 
 # Run tests matching a pattern
-hereby runtests --tests=tests/cases/fourslash/completion*.ts
+npx hereby runtests --tests=tests/cases/fourslash/completion*.ts
 ```
 
 ## Important Guidelines
 
+### Keeping Things Tidy
+
+- Once you think you're done, run `npx hereby lint` and fix any issues
+- Then always run `npx hereby format` as your last step
+
 ### Test Locations
+
 - Only add testcases in `tests/cases/compiler` or `tests/cases/fourslash`
+- Filenames in `tests/cases/compiler` must always end with `.ts`, not `.d.ts`
 - Do not write direct unit tests as they are almost never the correct test format for our repo
 
 ### Performance Expectations
+
 - Running a set of tests may take up to 4 minutes
 - A full test run may take up to 15 minutes
-- Always run `hereby lint` and `hereby format` before you're done
 
 ### Working with Issues
+
 - Maintainer comments in the issue should generally take priority over OP's comments
 - Maintainers might give you hints on where to start. They are not always right, but a good place to start
 
+### Debugging Tips
+
+printf debugging is going to be very useful as you are figuring things out.
+To do this, use `console.log`, but you'll need to `ts-ignore` it.
+Write something like this:
+```ts,diff
+function checkSomething(n: Node) {
+    doSomething(n);
++   // @ts-ignore DEBUG CODE ONLY, REMOVE ME WHEN DONE
++   console.log(`Got node with pos = ${n.pos}`);
+    doSomethingElse(n);
+}
+```
+We have a lot of enums so you might want to print back their symbolic name, to do this, index back into the name of the enum
+```ts
+   // @ts-ignore DEBUG CODE ONLY, REMOVE ME WHEN DONE
+   console.log(`Got node with kind = ${SyntaxKind[n.kind]}`);
+```
+
 ## Recommended Workflow
 
 When fixing bugs or implementing features, follow this workflow:
@@ -287,6 +313,15 @@ When fixing bugs or implementing features, follow this workflow:
    - Ensure the baselines change in a way that demonstrates that the bug is fixed
    - Put this baseline diff in its own commit
 
-4. **Run all other tests to ensure you didn't break anything**
-   - Some collateral baseline changes are normal
+4. **Add more testing**
+   - Once you've got the basics figured out, enhance your test to cover edge cases and other variations
+   - Run the test again and commit the baseline diff along with the test edit
+
+5. **Run all other tests to ensure you didn't break anything**
+   - Run `npx hereby runtests-parallel` and wait for it to finish (10-15 minutes is normal!)
+   - Some collateral baseline changes are normal, but review for correctness
    - Put these diffs in another commit
+
+6. **Always format and lint**
+   - Don't forget to run `npx hereby lint` and `npx hereby format` before you're done
+   - Double-check your line endings. Source files in this repo typically use CRLF line endings. Fix all line endings to be consistent before you wrap up

.github/workflows/copilot-setup-steps.yml

@@ -0,0 +1,22 @@
+name: 'Copilot Setup Steps'
+on: workflow_dispatch
+
+jobs:
+  # The job MUST be called `copilot-setup-steps` or it will not be picked up by Copilot.
+  copilot-setup-steps:
+    runs-on: ubuntu-latest
+
+    # Set the permissions to the lowest permissions possible needed for your steps.
+    # Copilot will be given its own token for its operations.
+    permissions:
+      # If you want to clone the repository as part of your setup steps, for example to install dependencies, you'll need the `contents: read` permission. If you don't clone the repository in your setup steps, Copilot will do this for you automatically after the steps complete.
+      contents: read
+
+    # You can define any steps you want, and they will run before the agent starts.
+    # If you do not check out your code, Copilot will do this for you.
+    steps:
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+      - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
+      - run: npm ci
+      # pull dprint caches before network access is blocked
+      - run: npx hereby check:format || true

src/compiler/checker.ts

@@ -25340,11 +25340,21 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         // right is a supertype.
         const superTypeOrUnion = literalTypesWithSameBaseType(primaryTypes) ?
             getUnionType(primaryTypes) :
-            reduceLeft(primaryTypes, (s, t) => isTypeSubtypeOf(s, t) ? t : s)!;
+            getSingleCommonSupertype(primaryTypes);
         // Add any nullable types that occurred in the candidates back to the result.
         return primaryTypes === types ? superTypeOrUnion : getNullableType(superTypeOrUnion, getCombinedTypeFlags(types) & TypeFlags.Nullable);
     }
 
+    function getSingleCommonSupertype(types: Type[]) {
+        // First, find the leftmost type for which no type to the right is a strict supertype, and if that
+        // type is a strict supertype of all other candidates, return it. Otherwise, return the leftmost type
+        // for which no type to the right is a (regular) supertype.
+        const candidate = reduceLeft(types, (s, t) => isTypeStrictSubtypeOf(s, t) ? t : s)!;
+        return every(types, t => t === candidate || isTypeStrictSubtypeOf(t, candidate)) ?
+            candidate :
+            reduceLeft(types, (s, t) => isTypeSubtypeOf(s, t) ? t : s)!;
+    }
+
     // Return the leftmost type for which no type to the right is a subtype.
     function getCommonSubtype(types: Type[]) {
         return reduceLeft(types, (s, t) => isTypeSubtypeOf(t, s) ? t : s)!;
@@ -53029,7 +53039,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             if (languageVersion < ScriptTarget.ES2015 && isPrivateIdentifier(node.name)) {
                 return grammarErrorOnNode(node.name, Diagnostics.Private_identifiers_are_only_available_when_targeting_ECMAScript_2015_and_higher);
             }
-            if (languageVersion < ScriptTarget.ES2015 && isAutoAccessorPropertyDeclaration(node)) {
+            if (languageVersion < ScriptTarget.ES2015 && isAutoAccessorPropertyDeclaration(node) && !(node.flags & NodeFlags.Ambient)) {
                 return grammarErrorOnNode(node.name, Diagnostics.Properties_with_the_accessor_modifier_are_only_available_when_targeting_ECMAScript_2015_and_higher);
             }
             if (isAutoAccessorPropertyDeclaration(node) && checkGrammarForInvalidQuestionMark(node.questionToken, Diagnostics.An_accessor_property_cannot_be_declared_optional)) {
@@ -53178,7 +53188,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         const literalType = isLiteralTypeNode(node.parent) ||
             isPrefixUnaryExpression(node.parent) && isLiteralTypeNode(node.parent.parent);
         if (!literalType) {
-            if (languageVersion < ScriptTarget.ES2020) {
+            // Don't error on BigInt literals in ambient contexts
+            if (!(node.flags & NodeFlags.Ambient) && languageVersion < ScriptTarget.ES2020) {
                 if (grammarErrorOnNode(node, Diagnostics.BigInt_literals_are_not_available_when_targeting_lower_than_ES2020)) {
                     return true;
                 }

src/compiler/parser.ts

@@ -4564,6 +4564,7 @@ namespace Parser {
             }
             parseExpected(SyntaxKind.ColonToken);
             attributes = parseImportAttributes(currentToken as SyntaxKind.WithKeyword | SyntaxKind.AssertKeyword, /*skipKeyword*/ true);
+            parseOptional(SyntaxKind.CommaToken);
             if (!parseExpected(SyntaxKind.CloseBraceToken)) {
                 const lastError = lastOrUndefined(parseDiagnostics);
                 if (lastError && lastError.code === Diagnostics._0_expected.code) {

src/compiler/utilities.ts

@@ -7715,9 +7715,18 @@ export function base64decode(host: { base64decode?(input: string): string; } | u
 export function readJsonOrUndefined(path: string, hostOrText: { readFile(fileName: string): string | undefined; } | string): object | undefined {
     const jsonText = isString(hostOrText) ? hostOrText : hostOrText.readFile(path);
     if (!jsonText) return undefined;
-    // gracefully handle if readFile fails or returns not JSON
-    const result = parseConfigFileTextToJson(path, jsonText);
-    return !result.error ? result.config : undefined;
+    // Try strictly parsing first, then fall back to our (slower)
+    // parser that is resilient to comments/trailing commas.
+    // package.json files should never have these, but we
+    // have no way to communicate these issues in the first place.
+    let result = tryParseJson(jsonText);
+    if (result === undefined) {
+        const looseResult = parseConfigFileTextToJson(path, jsonText);
+        if (!looseResult.error) {
+            result = looseResult.config;
+        }
+    }
+    return result;
 }
 
 /** @internal */

tests/baselines/reference/accessorInAmbientContextES5.errors.txt

@@ -0,0 +1,32 @@
+accessorInAmbientContextES5.ts(25,14): error TS18045: Properties with the 'accessor' modifier are only available when targeting ECMAScript 2015 and higher.
+
+
+==== accessorInAmbientContextES5.ts (1 errors) ====
+    // Should allow accessor in ambient contexts even when targeting ES5
+    
+    declare class AmbientClass {
+        accessor prop1: string;
+        static accessor prop2: number;
+        private accessor prop3: boolean;
+        private static accessor prop4: symbol;
+    }
+    
+    declare namespace AmbientNamespace {
+        class C {
+            accessor prop: string;
+        }
+    }
+    
+    // Should also work in .d.ts files (simulated with declare)
+    declare module "some-module" {
+        export class ExportedClass {
+            accessor value: any;
+        }
+    }
+    
+    // Regular class should still error when targeting ES5
+    class RegularClass {
+        accessor shouldError: string; // Should still error
+                 ~~~~~~~~~~~
+!!! error TS18045: Properties with the 'accessor' modifier are only available when targeting ECMAScript 2015 and higher.
+    }

tests/baselines/reference/accessorInAmbientContextES5.js

@@ -0,0 +1,81 @@
+//// [tests/cases/compiler/accessorInAmbientContextES5.ts] ////
+
+//// [accessorInAmbientContextES5.ts]
+// Should allow accessor in ambient contexts even when targeting ES5
+
+declare class AmbientClass {
+    accessor prop1: string;
+    static accessor prop2: number;
+    private accessor prop3: boolean;
+    private static accessor prop4: symbol;
+}
+
+declare namespace AmbientNamespace {
+    class C {
+        accessor prop: string;
+    }
+}
+
+// Should also work in .d.ts files (simulated with declare)
+declare module "some-module" {
+    export class ExportedClass {
+        accessor value: any;
+    }
+}
+
+// Regular class should still error when targeting ES5
+class RegularClass {
+    accessor shouldError: string; // Should still error
+}
+
+//// [accessorInAmbientContextES5.js]
+// Should allow accessor in ambient contexts even when targeting ES5
+var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, state, kind, f) {
+    if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a getter");
+    if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot read private member from an object whose class did not declare it");
+    return kind === "m" ? f : kind === "a" ? f.call(receiver) : f ? f.value : state.get(receiver);
+};
+var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, state, value, kind, f) {
+    if (kind === "m") throw new TypeError("Private method is not writable");
+    if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a setter");
+    if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot write private member to an object whose class did not declare it");
+    return (kind === "a" ? f.call(receiver, value) : f ? f.value = value : state.set(receiver, value)), value;
+};
+var _RegularClass_shouldError_accessor_storage;
+// Regular class should still error when targeting ES5
+var RegularClass = /** @class */ (function () {
+    function RegularClass() {
+        _RegularClass_shouldError_accessor_storage.set(this, void 0);
+    }
+    Object.defineProperty(RegularClass.prototype, "shouldError", {
+        get: function () { return __classPrivateFieldGet(this, _RegularClass_shouldError_accessor_storage, "f"); } // Should still error
+        ,
+        set: function (value) { __classPrivateFieldSet(this, _RegularClass_shouldError_accessor_storage, value, "f"); },
+        enumerable: false,
+        configurable: true
+    });
+    return RegularClass;
+}());
+_RegularClass_shouldError_accessor_storage = new WeakMap();
+
+
+//// [accessorInAmbientContextES5.d.ts]
+declare class AmbientClass {
+    accessor prop1: string;
+    static accessor prop2: number;
+    private accessor prop3;
+    private static accessor prop4;
+}
+declare namespace AmbientNamespace {
+    class C {
+        accessor prop: string;
+    }
+}
+declare module "some-module" {
+    class ExportedClass {
+        accessor value: any;
+    }
+}
+declare class RegularClass {
+    accessor shouldError: string;
+}

tests/baselines/reference/accessorInAmbientContextES5.symbols

@@ -0,0 +1,51 @@
+//// [tests/cases/compiler/accessorInAmbientContextES5.ts] ////
+
+=== accessorInAmbientContextES5.ts ===
+// Should allow accessor in ambient contexts even when targeting ES5
+
+declare class AmbientClass {
+>AmbientClass : Symbol(AmbientClass, Decl(accessorInAmbientContextES5.ts, 0, 0))
+
+    accessor prop1: string;
+>prop1 : Symbol(AmbientClass.prop1, Decl(accessorInAmbientContextES5.ts, 2, 28))
+
+    static accessor prop2: number;
+>prop2 : Symbol(AmbientClass.prop2, Decl(accessorInAmbientContextES5.ts, 3, 27))
+
+    private accessor prop3: boolean;
+>prop3 : Symbol(AmbientClass.prop3, Decl(accessorInAmbientContextES5.ts, 4, 34))
+
+    private static accessor prop4: symbol;
+>prop4 : Symbol(AmbientClass.prop4, Decl(accessorInAmbientContextES5.ts, 5, 36))
+}
+
+declare namespace AmbientNamespace {
+>AmbientNamespace : Symbol(AmbientNamespace, Decl(accessorInAmbientContextES5.ts, 7, 1))
+
+    class C {
+>C : Symbol(C, Decl(accessorInAmbientContextES5.ts, 9, 36))
+
+        accessor prop: string;
+>prop : Symbol(C.prop, Decl(accessorInAmbientContextES5.ts, 10, 13))
+    }
+}
+
+// Should also work in .d.ts files (simulated with declare)
+declare module "some-module" {
+>"some-module" : Symbol("some-module", Decl(accessorInAmbientContextES5.ts, 13, 1))
+
+    export class ExportedClass {
+>ExportedClass : Symbol(ExportedClass, Decl(accessorInAmbientContextES5.ts, 16, 30))
+
+        accessor value: any;
+>value : Symbol(ExportedClass.value, Decl(accessorInAmbientContextES5.ts, 17, 32))
+    }
+}
+
+// Regular class should still error when targeting ES5
+class RegularClass {
+>RegularClass : Symbol(RegularClass, Decl(accessorInAmbientContextES5.ts, 20, 1))
+
+    accessor shouldError: string; // Should still error
+>shouldError : Symbol(RegularClass.shouldError, Decl(accessorInAmbientContextES5.ts, 23, 20))
+}