Skip to content

Conversation

@gabritto
Copy link
Member

In Strada, identifiers had symbols, and when we synthesized an identifier in a type node, we'd store a symbol in it, which we would later use in inlay hints to obtain a location for an identifier in the inlay hint label parts.
This PR adapts that for Corsa, but instead of attaching a symbol to an identifier, we now have to store this information in a separate map that is provided to the node builder implementation.

All inlay hint existing diffs are now good changes as far as I can tell. The newly accepted diffs are due to a few cases where Strada couldn't provide a symbol for an identifier but Corsa can.

I also had to add code to avoid converting bundled file names to URIs in the tests, as that would cause a crash, and I left a !!! for possibly avoiding type node reuse in inlay hints once that's implemented, because I think otherwise we can't easily keep track of the mapping of identifiers to symbols.

@gabritto gabritto marked this pull request as ready for review December 30, 2025 21:55
Copilot AI review requested due to automatic review settings December 30, 2025 21:55
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 providing identifier locations in inlay hints, adapting the Strada approach to Corsa. Instead of attaching symbols directly to identifiers, the implementation uses a separate idToSymbol map that is provided to the node builder, which then tracks synthesized identifiers and their associated symbols. This enables inlay hints to provide location information for type identifiers, making them clickable in editors.

Key Changes

  • Added infrastructure to track symbol-to-identifier mappings through the node builder API
  • Enhanced inlay hints to include location information for identifiers in type annotations
  • Added special handling for bundled file URIs to prevent crashes during tests

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/checker/nodebuilder.go Added NewNodeBuilderEx and getNodeBuilderEx functions to support optional idToSymbol map parameter
internal/checker/nodebuilderimpl.go Added idToSymbol field, newIdentifier helper method, and logic to track symbol associations for synthesized identifiers
internal/checker/printer.go Updated TypeToTypeNode and TypePredicateToTypePredicateNode signatures to accept idToSymbol map parameter
internal/ls/inlay_hints.go Modified to create and pass idToSymbol map when generating type nodes, and updated to use map for location lookups
internal/ls/completions.go Updated call site to pass nil for idToSymbol parameter; removed unused ptrIsTrue and ptrIsFalse helper functions
internal/bundled/embed.go Added IsBundled function to detect bundled file paths
internal/bundled/noembed.go Added IsBundled function stub returning false
internal/lsp/lsproto/lsp.go Added check in FileName() to handle bundled URIs without conversion
internal/ls/lsconv/converters.go Added check in FileNameToDocumentURI() to handle bundled file names
internal/fourslash/fourslash.go Fixed baseline normalization to reset both start and end positions for lib file locations
internal/fourslash/tests/inlayHintsIdentifierLocation_test.go Added new test for identifier location feature
Test baselines Updated to reflect new location information in inlay hints

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.

2 participants