Skip to content

Conversation

@ritesh006
Copy link
Contributor

Summary
This PR adds support for showing "implementations" CodeLens on TypeScript methods that are overridden in derived classes.

Changes
Extended TypeScriptImplementationsCodeLensProvider to surface implementations for:
Class methods (not just interfaces or abstract methods)
Overridden methods in derived classes
Updated extractSymbol logic to correctly identify these symbols
Added smoke test: implementationsCodeLens.test.ts to validate behavior
Why
Resolves #198387.

Developers can now see "1 implementation" / "n implementations" CodeLens above methods that are overridden, and navigate directly to their derived class implementations.
This aligns CodeLens behavior with the “Find All Implementations” command.

Verification
Open an abstract/base class with a method that is overridden in derived classes.
Confirm that the "implementations" CodeLens appears above the method.
Clicking it navigates to the derived class overrides.
Testing
Local build & compile successful
Hygiene checks passed
Smoke test added & executed
Integration tests run successfully (scripts/test-integration.bat)
Notes
This improvement makes CodeLens more consistent and increases productivity when working with object-oriented TypeScript codebases.

Fixes #198387

@ritesh006
Copy link
Contributor Author

Hi team 👋
This PR implements “implementations” CodeLens for overridden methods (#198387).

Local tests ✅

Integration tests ✅

Hygiene checks ✅
Looking forward to your review. Please approve workflows when possible so CI can run.
Thanks!

@mjbvz mjbvz added this to the September 2025 milestone Sep 2, 2025
@ritesh006
Copy link
Contributor Author

could someone review this please

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 4, 2025

@ritesh006 We are currently preparing the August release. This PR came in too late to make it in but I've scheduled it for September. Will review it next week when I have a chance

Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Looks like the code change may have been lost? I only see the new test file and a single new line change

@ritesh006 ritesh006 force-pushed the test/implementations-codelens branch from 53cc94d to 21cbca8 Compare September 6, 2025 05:46
@ritesh006
Copy link
Contributor Author

Thanks for the catch, @mjbvz a merge from main had dropped the provider changes. I’ve restored them and force pushed. The PR now includes the updates in implementationsCodeLens.ts

Copy link

@jabari-max jabari-max left a comment

Choose a reason for hiding this comment

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

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 8, 2025

I'm still only seeing 1 file changed. Can you try again to make sure the other files are included

Also no need to explicitly keep copying me

@ritesh006 ritesh006 force-pushed the test/implementations-codelens branch 6 times, most recently from a2dd3f6 to 85cc11c Compare September 9, 2025 13:11
@ritesh006
Copy link
Contributor Author

Ready for review

}

private getCommand(locations: vscode.Location[], codeLens: ReferencesCodeLens): vscode.Command | undefined {
if (!locations.length) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We still want to show 0 implementations in this case. If you want to avoid showing the code lens for methods with no overrides, that should be handled differently

@ritesh006 ritesh006 force-pushed the test/implementations-codelens branch from 5437400 to 2a6a06a Compare September 9, 2025 17:26
@ritesh006
Copy link
Contributor Author

Ready for reviw

@ritesh006 ritesh006 force-pushed the test/implementations-codelens branch 2 times, most recently from f1f2365 to f73e56b Compare September 10, 2025 12:56
@ritesh006
Copy link
Contributor Author

Ready for Review.

}

// Class methods (behind new setting; default off)
if (
Copy link
Collaborator

@mjbvz mjbvz Sep 10, 2025

Choose a reason for hiding this comment

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

What about the other types that were previously in the switch?

@ritesh006 ritesh006 force-pushed the test/implementations-codelens branch 5 times, most recently from 3407cb2 to c0e26f7 Compare September 11, 2025 03:48
@ritesh006
Copy link
Contributor Author

Hi @mjbvz ready for review please have a look

}
break;
if (
(item.kind === PConst.Kind.method
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like class got dropped from this list

parent?.kind === PConst.Kind.class &&
cfg.get<boolean>('implementationsCodeLens.showOnClassMethods', false)
) {
return getSymbolRange(document, item);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should skip showing these on private methods as these cannot be overridden

@ritesh006
Copy link
Contributor Author

HI @mjbvz due to family loss I went to my hometown. as soon as I reach to my place I would like to start work again.

@ritesh006 ritesh006 force-pushed the test/implementations-codelens branch 2 times, most recently from 1202b7b to c0e26f7 Compare September 26, 2025 07:15
@ritesh006
Copy link
Contributor Author

Hi @mjbvz Ready for review

@mjbvz mjbvz modified the milestones: September 2025, October 2025 Sep 26, 2025
@mjbvz
Copy link
Collaborator

mjbvz commented Sep 26, 2025

Thanks, looks like there are still a few unresolved comments: https://github.com/microsoft/vscode/pull/264546/files#diff-2c8c54699e981dcd79e99a1b8cbc2690c7b751ac4b160fe7fd3f271835b5b878

Can you please take a look a those

Since it's a little late for the September release, we'll also target the October release for getting this merged

@ritesh006 ritesh006 force-pushed the test/implementations-codelens branch from c0e26f7 to 4b77716 Compare October 1, 2025 11:38
@ritesh006
Copy link
Contributor Author

Hi @mjbvz Ready for review

@ritesh006
Copy link
Contributor Author

Hi @mjbvz Ready for review

please let me know if anything I missed and so sorry for late reply

@ritesh006 ritesh006 force-pushed the test/implementations-codelens branch from 67b5546 to cd5348a Compare October 11, 2025 05:19
@ritesh006
Copy link
Contributor Author

Ready For Review @mjbvz

@mjbvz mjbvz enabled auto-merge October 15, 2025 07:40
@mjbvz
Copy link
Collaborator

mjbvz commented Oct 15, 2025

Added some extra tests to have more confidence in this change. Also renamed it to showOnAllClassMethods

mjbvz
mjbvz previously approved these changes Oct 15, 2025
@bpasero
Copy link
Member

bpasero commented Oct 15, 2025

@mjbvz red CI

alexr00
alexr00 previously approved these changes Oct 15, 2025
@mjbvz mjbvz dismissed stale reviews from alexr00 and themself via 40fd1d8 October 15, 2025 17:16
@mjbvz mjbvz merged commit 078d0d5 into microsoft:main Nov 3, 2025
17 checks passed
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.

Show "implementations" code lens for TypeScript "virtual"/overridden methods

6 participants