Skip to content

Conversation

@gabritto
Copy link
Member

@gabritto gabritto commented Sep 10, 2025

Port of inlay hints, mostly the same as Strada, plus a bug fix in node builder when setting raw text of template literal types.
Notable differences:

  • Removed interactiveInlayHints. This used to control whether we used strings or inlay hint label parts, and was hardcoded as true in vscode. If we need to optimize the performance of this in the future, we could resolve a label's location lazily, which is the thing that could most significantly affect performance, depending on how it's implemented.
  • We don't compute label locations yet in Corsa, because the way we used to do it in Strada relied on symbols stored in identifiers, and we don't have that in Corsa anymore, and there's no trivial way to get it. Leaving this for a future PR, and the non-submodule accepted diffs are all because of that.

texts := t.AsTemplateLiteralType().texts
types := t.AsTemplateLiteralType().types
templateHead := b.f.NewTemplateHead(texts[0], texts[0], ast.TokenFlagsNone)
templateHead := b.f.NewTemplateHead(texts[0], "", ast.TokenFlagsNone)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fun, maybe we should just quick pull this into main?

case ast.KindIdentifier:
identifierText := node.Text()
var name *ast.Node
// !!! This won't work in Corsa since we don't store symbols on identifiers. We need another strategy for it.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effect of this is that you can't hover over an identifier in an inlay hint in the editor. But I'm leaving this to be fixed later, because it seems more important to have inlay hints working in the editor now, and it could take some non-trivial work to fix this, e.g. constructing a map of identifiers to symbols in the node builder.

conformance/node10Alternateresult_noTypes.errors.txt.diff
conformance/node10AlternateResult_noResolution.errors.txt.diff
conformance/node10AlternateResult_noResolution.errors.txt.diff
fourslash/inlayHints/inlayHintsInteractiveParameterNamesWithComments.baseline.diff
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff is because I fixed a bug from Strada where we didn't fully detect if a comment had the parameter name.

conformance/node10AlternateResult_noResolution.errors.txt.diff
fourslash/inlayHints/inlayHintsInteractiveParameterNamesWithComments.baseline.diff
fourslash/inlayHints/inlayHintsImportType2.baseline.diff
fourslash/inlayHints/inlayHintsInteractiveImportType2.baseline.diff
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two are due to a good change in the checker when we check JavaScript expandos: they used to be checked in a special way, but are now checked the same as TS expressions.

fourslash/inlayHints/inlayHintsInteractiveParameterNamesWithComments.baseline.diff
fourslash/inlayHints/inlayHintsImportType2.baseline.diff
fourslash/inlayHints/inlayHintsInteractiveImportType2.baseline.diff
fourslash/inlayHints/inlayHintsFunctionParameterTypes1.baseline.diff
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diffs from here on are due to removing the interactiveInlayHints option, which is now always true in Corsa.

@gabritto gabritto marked this pull request as ready for review November 6, 2025 18:30
Copilot AI review requested due to automatic review settings November 6, 2025 18:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for inlay hints functionality in the TypeScript language server port. The main change introduces a DiffFixupNew function alongside the existing DiffFixupOld to allow transformations on baseline output during test comparisons.

Key Changes

  • Added DiffFixupNew field to baseline test options for transforming actual output before comparison
  • Updated baseline diff generation to apply both old and new fixup functions
  • Accepted 23 new fourslash inlay hints test baselines showing convergence with TypeScript's output format (labels now use structured arrays instead of plain strings)
  • Fixed template literal escape sequence handling in types (e.g., \t and \r\n now properly preserved instead of being expanded)

Reviewed Changes

Copilot reviewed 165 out of 165 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
testdata/submoduleAccepted.txt Added 23 inlay hints test files to accepted baseline differences
testdata/baselines/reference/submoduleAccepted/fourslash/inlayHints/*.baseline.diff Accepted baseline diffs showing inlay hint label format changes from strings to structured arrays
testdata/baselines/reference/submodule/fourslash/inlayHints/*.baseline New baseline files for inlay hints tests with correct structured label format
testdata/baselines/reference/submodule/compiler/templateLiteralsInTypes.* Fixed template literal escape sequences in type representations
internal/testutil/baseline/baseline.go Added DiffFixupNew option for transforming actual output in baseline comparisons

@gabritto gabritto requested a review from jakebailey November 6, 2025 20:56
^
{
- "label": ": string",
+ "label": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For baselining purposes, should we convert these to plain strings?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the baselines or for diffing the baselines?

+ "label": [
+ {
+ "value": "a",
+ "location": {},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Location is empty in baselines; are we missing some code that can baseline these?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Given we didn't baseline them before, I do wonder if we should just not bother look at this in baselines to have fewer diffs)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We did baseline them before, the thing is in Strada we use position + file, and in Corsa it's LSP location, and I didn't want to write code to convert one into the other, I figured for now the important part is to notice if we do have a location or not, so I'm just emptying locations in the diff fixup functions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, but there's only a string here, not something with a location? Maybe I'm missing something

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This label stands for the identifier a, so its value is "a", and the location should point to the original parameter identifier, but location fields are being omitted when computing the diffs. They do show up in the .baseline file. If there's something I can do to make this all less confusing, let me know, I haven't put a ton of effort into the diff fixing code because we're eventually getting rid of it.

Copy link
Member

@jakebailey jakebailey Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the thing I was missing was that the Strada baseline says:

{
  "text": "a:",
  "position": 88,
  "kind": "Parameter",
  "whitespaceAfter": true
}

Whereas the new baseline says:

{
  "position": {
    "line": 6,
    "character": 6
  },
  "label": [
    {
      "value": "a",
      "location": {
        "uri": "file:///a.js",
        "range": {
          "start": {
            "line": 3,
            "character": 17
          },
          "end": {
            "line": 3,
            "character": 18
          }
        }
      }
    },
    {
      "value": ":"
    }
  ],
  "kind": 2,
  "paddingRight": true
}

So with that context, it makes sense what is happening here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, that's because "interactive inlay hints" is always true in Corsa now, so we always return inlay hint label parts.

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skimming, I think this seems good. All of the baseline diffs I read look good, and even though VS Code enables it by default, the defaults are such that they won't actually turn on until the user turns on one of the typescript.inlayHints. settings, so for checker.ts this costs nothing.

For the people who turn this on, we're going to use increased resources, but I think this is not a bad situation given that VS code doesn't enable these.

@gabritto gabritto added this pull request to the merge queue Nov 7, 2025
Merged via the queue into main with commit db968bc Nov 7, 2025
22 checks passed
@gabritto gabritto deleted the gabritto/inlayhints branch November 7, 2025 03:27
nathanwhit added a commit to denoland/typescript-go that referenced this pull request Dec 2, 2025
* Port 'go to type definition' tests (microsoft#1883)

* Fix panic in `getTokenAtPosition` for JSDoc type assertions (microsoft#1846)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>
Co-authored-by: andrewbranch <[email protected]>
Co-authored-by: Andrew Branch <[email protected]>

* Don’t look in JSExportAssignment and CommonJSExport for nodes (microsoft#1886)

* Fix link in native preview platform packages (microsoft#1838)

* fix(1880): No error message for JSDoc type parsing (microsoft#1881)

* Add vscode editor issue template (microsoft#1893)

Co-authored-by: Ryan Cavanaugh <[email protected]>

* Add "Report Issue" button to TSGO status bar commands (microsoft#1889)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: DanielRosenwasser <[email protected]>
Co-authored-by: Daniel Rosenwasser <[email protected]>

* fix(1898): adjust location handling in find-refs (microsoft#1901)

* Fix panic of empty string in type reference directive (microsoft#1908)

* Consistently error on full circle of circular import aliases (microsoft#1904)

* Fix panic in textDocument/onTypeFormatting when tokenAtPosition is nil (microsoft#1845)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Update submodule (microsoft#1913)

* Disable create-cache.yml in forks (microsoft#1912)

* Forbid platform specific package uses in agnostic files (microsoft#1911)

* Fix JSDoc comment formatting with tab indentation (microsoft#1900)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Clear local baseline dir in hereby test (microsoft#1921)

* Unskip passing fourslash test (microsoft#1922)

* Support auto-import completion fourslash tests, fix bugs (microsoft#1917)

* Fix JSX indentation in JavaScript output (microsoft#1792)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Implement printAllHelp to fix `tsgo --all` producing no output (microsoft#1843)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Bump the github-actions group across 1 directory with 2 updates (microsoft#1909)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jake Bailey <[email protected]>

* Ensure os package is forbidden in lint (microsoft#1924)

* Speed up levenshteinWithMax by reusing buffers (microsoft#1823)

* Fix incorrect formatting for comments inside multi-line argument lists and method chains (microsoft#1929)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Handle nil end position in getMappedLocation (microsoft#1920)

* Fix formatter adding extra space at end of line without trailing newline (microsoft#1933)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Fix vscode issue template (microsoft#1934)

* userpreferences parsing/ls config handing (microsoft#1729)

* Plumb through TokenFlagsSingleQuote; use for auto import quote detection (microsoft#1937)

* Invalidate caches on batches of 1000+ watch changes (microsoft#1869)

* Create clickable links in quick info from @link JSDoc tags (microsoft#1935)

* Don't report errors on `{@link foo.bar}` references (microsoft#1941)

* Fix crash in `invocationErrorRecovery` function (microsoft#1944)

* Fix leading source file comment emit bugs (microsoft#1945)

* Implement selection ranges (microsoft#1939)

* Fix porting bug in isArgumentAndStartLineOverlapsExpressionBeingCalled (microsoft#1948)

* Add Range to Hover (microsoft#1489)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: DanielRosenwasser <[email protected]>
Co-authored-by: Jake Bailey <[email protected]>

* Properly handle hovering on `this` (microsoft#1953)

* Bump the github-actions group across 1 directory with 2 updates (microsoft#1959)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fixed document highlight for reference directive (microsoft#1951)

* Several fixes to JS typing of functions and methods (microsoft#1960)

* Update submodule, port 6.0 options defaults (microsoft#1961)

* Reapply microsoft#1951 and microsoft#1960 after bad merge (microsoft#1964)

Co-authored-by: Anders Hejlsberg <[email protected]>
Co-authored-by: John Favret <[email protected]>

* Update submodule with ES5 removals (microsoft#1963)

* Actually transform KindCommonJSExport in declaration emit (microsoft#1962)

* Quick Info fixes (microsoft#1971)

* Fix various named enum types (microsoft#1973)

* Move change tracker, converters, utils to separate packages (microsoft#1977)

* Consistent rules for mixing `@type`, `@param`, `@return`, `@template` (microsoft#1979)

* Check for identifier before obtaining text of name (microsoft#1984)

* Respect client capabilities for diagnostics (microsoft#1980)

* Store explicitly declared members ahead of inherited members (microsoft#1987)

* Add --checkers to control number of checkers per Program (microsoft#1985)

* Export all types referenced through other exported APIs, enforce (microsoft#1978)

* Switch custom runners from mariner-2.0 to azure-linux-3 (microsoft#1989)

* Use `LocationLink` in go to definition (microsoft#1884)

* Use a different set of commands to detect fourslash test updates (microsoft#1923)

* Skip erasableSyntaxOnly checks for JavaScript files (microsoft#1956)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Update submodule for new cherry-picks (microsoft#1996)

* Port TypeScript PR #62604: Propagate variance reliability (microsoft#1916)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: RyanCavanaugh <[email protected]>
Co-authored-by: Jake Bailey <[email protected]>

* Only export `@typedef` type aliases in modules (microsoft#1999)

* Bump github/codeql-action from 4.31.0 to 4.31.2 in the github-actions group across 1 directory (microsoft#2005)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Hoist @typedef and @import tags to containing scopes that permit them (microsoft#2003)

* Remove concept of "unsupported extensions", clean up test skips  (microsoft#2004)

* Update golangci-lint, fix issues, modernize (microsoft#1981)

* Implement more handling of client capabilities (microsoft#1998)

* Add docs to signature help (microsoft#2009)

Co-authored-by: Copilot <[email protected]>

* Fix unused identifier diags, LSP tag diags (microsoft#2007)

* fix(2015): abstract property created, overshadowing override (microsoft#2016)

* Always check refCount after acquiring lock (microsoft#1986)

* Delete resolver unit tests (microsoft#2008)

* Fix missing parent for `Expression` in `TypeParameterDeclaration` (microsoft#2017)

* Add missing nil check in `getCompletionItemActions` (microsoft#2018)

* Fix crash in find-all-refs on `exports.xxx` in .js file (microsoft#2023)

* Properly include JSX attributes in find-all-references (microsoft#2025)

* Fix crash by removing `getNameFromImportDeclaration` in favor of `Node.Name()` (microsoft#2027)

* Fix losing options from command line in watch mode (microsoft#2024)

* Add issue investigator agent (microsoft#2030)

* Switch 1ESPT pipelines to 1ESPT-AzureLinux3 (microsoft#2031)

* Port inlay hints (microsoft#1705)

* Split "use strict" into separate transformer, fix bugs with prologues (microsoft#2028)

Co-authored-by: Sheetal Nandi <[email protected]>

* Use a more cross-architecture-friendly devcontainer image. (microsoft#2034)

* Fix nil pointer dereference in range formatting (microsoft#1993)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: DanielRosenwasser <[email protected]>
Co-authored-by: jakebailey <[email protected]>
Co-authored-by: Daniel Rosenwasser <[email protected]>
Co-authored-by: Copilot <[email protected]>

* Port missing `checkJs` logic (microsoft#2046)

* Ignore reparsed nodes when determining external module indicator (microsoft#2044)

* Fix various fuzzer-caught crashes in the parser (microsoft#2038)

* Fix moduleDetection for node18, fix __esModule in detect=force (microsoft#2045)

* Fix panic in syncmap on loading nil (microsoft#2056)

* Use accessors on `Node` instead of casts and field accesses (microsoft#2052)

* Add locks on concurrent alias following checker accesses under incremental mode (microsoft#2051)

* Don't add `export` modifier to `JSTypeAliasDeclaration` from `@callback` (microsoft#2063)

* Introduce GetECMALineOfPosition to avoid unused rune counting (microsoft#2065)

* Don't add `export` modifier to `KindCommonJSExport` reparsed nodes (microsoft#2066)

* Fix panic in inlay hints for tuple types (microsoft#2040)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Accurately recognize fourslash test as submodule (microsoft#2068)

* Implement auto-import code actions, port tests and fix some bugs (microsoft#2053)

* Port tsc --init (microsoft#2033)

* Make CheckerPool iteration concurrent by default (microsoft#2070)

* Use Microsoft build of Go in CI (microsoft#2069)

* Detect Windows junctions with GetFileAttributesEx (microsoft#2013)

* Fix CI cache workflow (microsoft#2071)

* Use information from contextual type in hovers/quick info (microsoft#2073)

* fix(2074): No quick info on function and other similar tokens (microsoft#2078)

* Unify locks used on checkers between exclusive pool borrows and EmitResolver scopes (microsoft#2080)

* Port non-baseline diagnostics tests (microsoft#2079)

* Use SkipTrivia instead of GetRangeOfTokenAtPosition where possible (microsoft#2089)

* Move unreachable checks to checker, allowing more AST reuse (microsoft#2067)

* Fix scanning of valid surrogate pairs (microsoft#2032)

* Fix misplaced parentheses in `Checker.isIndirectCall` (microsoft#2093)

* Various agent mode updates (microsoft#2094)

* Handle configuration changes in LSP for 'typescript.*' options. (microsoft#2088)

* Fix nil pointer dereference in getAdjustedLocation for type-only exports (microsoft#2090)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Fix nil pointer dereference in code actions when diagnostic code is nil (microsoft#2091)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jakebailey <[email protected]>

* Fully resolve LSP client caps to non-pointers, pass by context (microsoft#2095)

* Fix hover on `module.exports` (microsoft#2098)

* Accept and document jsdoc diffs, round 1 (microsoft#1426)

Co-authored-by: Copilot <[email protected]>
Co-authored-by: Jake Bailey <[email protected]>

* Port baseline diagnostics tests (microsoft#2097)

* Clean up disk space in CI before running (microsoft#2103)

Co-authored-by: Copilot <[email protected]>

* Add GOBIN to PATH in CI (microsoft#2105)

* Make client requests type safe, unmarshal (microsoft#2099)

* Display inherited JSDoc documentation in quick info (microsoft#2111)

* Sort failingTests and manualTests in en-US (microsoft#2113)

* fix(2047): Incomplete declaration emit of callback tag with no return tag (microsoft#2100)

* Fix canHaveSyntheticDefault (microsoft#2101)

* Misc fixes (microsoft#2112)

* chore: fix incorrect function name in comment (microsoft#2109)

Signed-off-by: weifangc <[email protected]>

* Fix typedef binding with CJS `exports=` (microsoft#826)

* Provide Program diagnostics as push diags in tsconfig.json (microsoft#2118)

Co-authored-by: Copilot <[email protected]>

* Update dependencies (microsoft#2116)

* Remove copilot-setup-steps env var (microsoft#2124)

* Fix panic on negative parameterIndex in type predicate flow analysis (microsoft#2122)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: RyanCavanaugh <[email protected]>
Co-authored-by: Ryan Cavanaugh <[email protected]>

* Port tests for go to implementation and diff definitions tests (microsoft#2130)

* Partially fix multi-checker diagnostics consistency (microsoft#2134)

* Implement reportStyleChecksAsWarnings (microsoft#2132)

* Include docs on resolved client caps (microsoft#2135)

* Fix dynamic import grammar check (microsoft#2138)

* Refine LSP with our own types, generate more stuff (microsoft#2141)

* Display all symbol meanings in quick info (microsoft#2144)

* Multiproject requests like find all refs, rename and workspace symbols (microsoft#1991)

* Add stringer-alike String methods to non-string LSP enums (microsoft#2148)

* Enable localization (microsoft#2123)

* Port workspace symbols tests (microsoft#2146)

* Update readme, issue template (microsoft#2140)

* Ignore config port (microsoft#1755)

* fix(2157): jsdocfunction param is inferred as implicit any when directly assigned to module.exports (microsoft#2158)

* fixes after merge

* some more fixes

* more errors

* builds

* fmt

* fix nil pointer deref

* fix error messages

---------

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: weifangc <[email protected]>
Co-authored-by: Gabriela Araujo Britto <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: jakebailey <[email protected]>
Co-authored-by: andrewbranch <[email protected]>
Co-authored-by: Andrew Branch <[email protected]>
Co-authored-by: Andrew Branch <[email protected]>
Co-authored-by: Oleksandr T. <[email protected]>
Co-authored-by: Matt Bierner <[email protected]>
Co-authored-by: Ryan Cavanaugh <[email protected]>
Co-authored-by: DanielRosenwasser <[email protected]>
Co-authored-by: Daniel Rosenwasser <[email protected]>
Co-authored-by: Twacqwq <[email protected]>
Co-authored-by: Anders Hejlsberg <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Isabel Duan <[email protected]>
Co-authored-by: John Favret <[email protected]>
Co-authored-by: Wesley Wigham <[email protected]>
Co-authored-by: RyanCavanaugh <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: xu0o0 <[email protected]>
Co-authored-by: Sheetal Nandi <[email protected]>
Co-authored-by: Nathan Shively-Sanders <[email protected]>
Co-authored-by: weifangc <[email protected]>
Co-authored-by: Ryan Cavanaugh <[email protected]>
Co-authored-by: Nathan Whitaker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants