findAllReferences inital port #882

iisaduan · 2025-05-20T07:15:48Z

This port focuses on the logic needed to resolving symbols within a single file.

Todos after this PR:

Search for references through module resolving
Search for references through imported and exported symbol resolving
- Some imported and global symbols return references, but it is hard to test with the current lstestutil capabilities.
Jsdoc references (not sure how much of the currently un-ported parts will be needed, since @sandersn has been working to simplify jsdoc checking)
Special cases for object literals
Special cases for classes and keywords
String references

…o findAllRef

Copilot

Pull Request Overview

This PR ports the findAllReferences functionality and updates various components across the codebase. Key changes include:

Adding new APIs and handlers (e.g. ResolveTripleslashReference in the path package and handleReferences in the LSP server) to support reference lookup.
Replacing usage of older APIs (like GetAssignmentDeclarationKind) with new ones (GetJSDocAssignmentDeclarationKind) and renaming several utility functions for better export visibility.
Introducing new testing files for findAllReferences and new core utilities (FlatMap and CoalesceList).

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/tspath/path.go	Added ResolveTripleslashReference for triple-slash path resolution.
internal/parser/reparser.go	Updated assignment declaration kind calls to use GetJSDocAssignmentDeclarationKind.
internal/lsp/server.go	Added handling for ReferenceParams and a new handleReferences function.
internal/ls/findallreferencesexport_test.go	New test file for export reference lookups.
internal/ls/findallreferences_test.go	New tests covering various findAllReferences scenarios.
internal/core/core.go	Introduced FlatMap and CoalesceList utility functions.
internal/compiler/program.go	Added functions for module resolution improvements and extension handling.
internal/checker/utilities.go	Renamed internal helper functions (e.g. HasModifier, GetSingleVariableOfVariableStatement).
internal/checker/services.go	Added GetSymbolsOfParameterPropertyDeclaration for parameter-property handling.
internal/checker/printer.go	Updated modifier check call in expression printing.
internal/checker/exports.go	Added exported helper functions for type operations and symbol merging.
internal/checker/checker.go	Replaced legacy modifier and assignment kind calls with their updated versions.
internal/binder/nameresolver.go	Updated internal module/enum checks and exported GetLocalSymbolForExportDefault.
internal/binder/binder.go	Switched to use GetJSDocAssignmentDeclarationKind in assignment binding logic.
internal/astnav/tokens.go	Added GetTouchingToken as an alternative for token retrieval in AST.
internal/ast/utilities.go	Introduced several new AST helper functions and renamed utility methods.

internal/core/core.go

internal/checker/services.go

internal/ast/utilities.go

internal/checker/utilities.go

internal/core/core.go

internal/ls/findallreferencesexport_test.go

internal/ls/utilities.go

internal/checker/checker.go

internal/ls/findallreferences.go

sandersn · 2025-05-21T17:43:18Z

internal/ast/utilities.go

@@ -1029,6 +1029,26 @@ func IsStatic(node *Node) bool {
 	return IsClassElement(node) && HasStaticModifier(node) || IsClassStaticBlockDeclaration(node)
 }

+func CanHaveSymbol(node *Node) bool {
+	switch node.Kind {


All the JSDoc tags should be dropped from here, replaced with JSExportAssignment JSTypeAliasDeclaration, CommonJSExport (I think).
Also I'm pretty sure Identifier is JS-only and unsupported in Corsa, so can be dropped as well.

sandersn · 2025-05-21T17:44:34Z

internal/ast/utilities.go

@@ -1290,6 +1314,37 @@ func IsThisParameter(node *Node) bool {
 	return IsParameter(node) && node.Name() != nil && IsThisIdentifier(node.Name())
 }



a lot of this syntax isn't supported, or supported yet, in Corsa. Merge fom main and use GetAssignmentDeclarationKind as-is.

sandersn

I haven't read findallreferences.go yet but here are my comments for the rest. I focussed on changes needed for JS. I'll follow up with comments for the big file tomorrow.

sandersn · 2025-05-21T17:46:17Z

internal/ast/utilities.go

+
+	// PropertyAccessKinds
+	// F.prototype = { ... }
+	JSDeclarationKindPrototype


JSDeclarationKindPrototype isn't supported in Corsa, and Object.defineProperty is at the very bottom of my TODO list. Same advice as above.

sandersn · 2025-05-21T17:49:28Z

internal/ast/utilities.go

@@ -1766,13 +1906,13 @@ func IsExpressionNode(node *Node) bool {
 		for node.Parent.Kind == KindQualifiedName {
 			node = node.Parent
 		}
-		return IsTypeQueryNode(node.Parent) || isJSDocLinkLike(node.Parent) || isJSXTagName(node)
+		return IsTypeQueryNode(node.Parent) || IsJSDocLinkLike(node.Parent) || isJSXTagName(node)


I have to wonder if I came up with the name IsJSDocLinkLike about the time I was reading Fox in Socks' "Luke Luck likes lakes, Luke's duck licks lakes" twice a day.

sandersn · 2025-05-21T17:50:06Z

internal/ast/utilities.go

@@ -2456,6 +2596,10 @@ func GetDeclarationContainer(node *Node) *Node {
 	}).Parent
 }

+func IsPrototypeAccess(node *Node) bool {


same advice as above

sandersn · 2025-05-21T17:51:41Z

internal/core/set.go

@@ -48,6 +48,17 @@ func (s *Set[T]) AddIfAbsent(key T) bool {
 	return true
 }

+// Returns true if the key is arleady in the set. Adds the key and returns false otherwise.


Suggested change

// Returns true if the key is arleady in the set. Adds the key and returns false otherwise.

// Returns true if the key is already in the set. Adds the key and returns false otherwise.

sandersn · 2025-05-21T17:57:20Z

internal/checker/checker.go

@@ -13673,7 +13673,7 @@ func (c *Checker) getSymbolOfPartOfRightHandSideOfImportEquals(entityName *ast.N
 	//     import a = |b|; // Namespace
 	//     import a = |b.c|; // Value, type, namespace
 	//     import a = |b.c|.d; // Namespace
-	if entityName.Kind == ast.KindIdentifier && isRightSideOfQualifiedNameOrPropertyAccess(entityName) {
+	if entityName.Kind == ast.KindIdentifier && IsRightSideOfQualifiedNameOrPropertyAccess(entityName) {


I love the way changing accessibility of a thing requires changing all references to it everywhere.

the gift that keeps on giving 😅

For this package, just declare the exported one in exports.go.

sandersn · 2025-05-21T22:35:05Z

internal/ls/utilities.go

+
+	for meaning != lastIterationMeaning {
+		// The result is order-sensitive, for instance if initialMeaning == Namespace, and declarations = [class, instantiated module]
+		// we need to consider both as they initialMeaning intersects with the module in the namespace space, and the module


Suggested change

// we need to consider both as they initialMeaning intersects with the module in the namespace space, and the module

// we need to consider both as the initialMeaning intersects with the module in the namespace space, and the module

sandersn · 2025-05-21T22:36:42Z

internal/ls/utilities.go

+	return res
+}
+
+/**


I think the Go format for doc comments is just //. Not certain though.

sandersn · 2025-05-21T22:37:51Z

internal/ls/utilities.go

+* @param result an array of symbol of found property symbols
+* @param previousIterationSymbolsCache a cache of symbol from previous iterations of calling this function to prevent infinite revisiting of the same symbol.
+*                                The value of previousIterationSymbol is undefined when the function is first called.


these don't appear to match the current parameters

sandersn · 2025-05-21T22:39:56Z

internal/ls/utilities.go

+	}
+
+	res := core.MapNonNil(
+		core.IfElse(lhsType.Flags() != 0, lhsType.Types(), core.IfElse(lhsType.Symbol() == symbol.Parent, nil, []*checker.Type{lhsType})),


this feels like it does too much work for a pair of IfElses, though it would be much longer to write out as if/elses.

sandersn · 2025-05-21T22:52:44Z

internal/ls/utilities.go

+	return nil
+}
+
+func getSourceOfDefaultedAssignment(node *ast.Node) *ast.Node {


not needed because this is called from unneeded code.
also, defaulted assignments (window.x = window.x || "thing") are not supported in Corsa

sandersn

Minor comments on findallreferences.go, nothing substantive.

sandersn · 2025-05-22T12:41:20Z

internal/ls/findallreferences.go

+		if ast.IsInJSFile(node) {
+			binaryExpression := core.IfElse(node.Parent.Kind == ast.KindBinaryExpression,
+				node.Parent,
+				core.IfElse(ast.IsAccessExpression(node.Parent) && node.Parent.Parent.Kind == ast.KindBinaryExpression && node.Parent.Parent.AsBinaryExpression().Left == node.Parent,


this branch might not be needed, but I can't tell. better to wait until we have tests to find out.

sandersn · 2025-05-22T12:49:01Z

internal/ls/findallreferences.go

+		return nil
+	}
+
+	if !ast.IsDeclaration(node.Parent) && node.Parent.Kind != ast.KindExportAssignment {


I'm not 100% sure this is needed, but:

Suggested change

if !ast.IsDeclaration(node.Parent) && node.Parent.Kind != ast.KindExportAssignment {

if !ast.IsDeclaration(node.Parent) && node.Parent.Kind != ast.KindExportAssignment && node.Parent.Kind != ast.KindJSExportAssignment {

internal/ls/findallreferences.go

sandersn · 2025-05-22T13:20:18Z

internal/ls/findallreferences.go

+	// If this is private property or method, the scope is the containing class
+	if symbol.Flags&(ast.SymbolFlagsProperty|ast.SymbolFlagsMethod) != 0 {
+		privateDeclaration := core.Find(declarations, func(d *ast.Node) bool {
+			return checker.HasModifier(d, ast.ModifierFlagsPrivate) || ast.IsPrivateIdentifierClassElementDeclaration(d)


aside--if this port didn't skip any Strada jsdoc checks for @private, then Corsa's going to be more a lot more accurate for JS, since my next PR is to convert @private into ModifierFlagsPrivate. (Nothing for you to do here, just an observation.)

sandersn · 2025-05-22T13:22:35Z

internal/ls/findallreferences.go

+		}
+
+		scope = container
+		if scope.Kind == ast.KindFunctionExpression {


Later: pretty sure this isn't needed.

sandersn · 2025-05-22T13:33:09Z

internal/ls/findallreferences.go

+		searchSpaceNode = searchSpaceNode.Parent // re-assign to be the owning class
+		break
+	case ast.KindSourceFile:
+		if ast.IsExternalModule(searchSpaceNode.AsSourceFile()) || isParameterName(thisOrSuperKeyword) {


I don't see how GetThisContainer could return a SourceFile for a parameter name but also this never fails to surprise me.

internal/ls/findallreferences.go

sandersn · 2025-05-22T13:44:31Z

internal/ls/findallreferences.go

+	// Use the parent symbol if the location is commonjs require syntax on javascript files only.
+	if ast.IsInJSFile(referenceLocation) && referenceLocation.Parent.Kind == ast.KindBindingElement &&
+		ast.IsVariableDeclarationInitializedToRequire(referenceLocation.Parent.Parent.Parent) {
+		// !!! when findAllReferences has been fully implemented, check if the behavior is the same since isVariableDeclarationInitializedToBareOrAccessedRequire has been removed


only the tests that use a property access on a require should fail, eg const x = require('mod').x

internal/ls/findallreferences.go

iisaduan added 4 commits May 15, 2025 16:04

initial

db09dbb

tests, some bug fixes, cleanup

95937fb

Merge branch 'main' of https://github.com/microsoft/typescript-go int…

e64221d

…o findAllRef

update to cancellation structure

9a25ad0

iisaduan requested review from sandersn, andrewbranch, jakebailey, gabritto and Copilot May 20, 2025 07:15

Copilot AI reviewed May 20, 2025

View reviewed changes

internal/core/core.go Show resolved Hide resolved

internal/checker/services.go Show resolved Hide resolved

gabritto reviewed May 20, 2025

View reviewed changes

iisaduan added 2 commits May 20, 2025 12:01

address comments

632e5fe

use set as node tracker

bb63fd6

jakebailey reviewed May 20, 2025

View reviewed changes

internal/ls/findallreferences.go Show resolved Hide resolved

sandersn reviewed May 21, 2025

View reviewed changes

sandersn requested changes May 21, 2025

View reviewed changes

sandersn requested changes May 22, 2025

View reviewed changes

jakebailey mentioned this pull request May 26, 2025

extension: rename symbol is not a thing anymore #938

Open

		@@ -1290,6 +1314,37 @@ func IsThisParameter(node *Node) bool {
		return IsParameter(node) && node.Name() != nil && IsThisIdentifier(node.Name())
		}

	// Returns true if the key is arleady in the set. Adds the key and returns false otherwise.
	// Returns true if the key is already in the set. Adds the key and returns false otherwise.

	// we need to consider both as they initialMeaning intersects with the module in the namespace space, and the module
	// we need to consider both as the initialMeaning intersects with the module in the namespace space, and the module

	if !ast.IsDeclaration(node.Parent) && node.Parent.Kind != ast.KindExportAssignment {
	if !ast.IsDeclaration(node.Parent) && node.Parent.Kind != ast.KindExportAssignment && node.Parent.Kind != ast.KindJSExportAssignment {

findAllReferences inital port #882

Are you sure you want to change the base?

findAllReferences inital port #882

Uh oh!

Conversation

iisaduan commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandersn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabritto May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandersn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iisaduan commented May 20, 2025 •

edited

Loading

gabritto May 21, 2025 •

edited

Loading