Skip to content

Commit c96b472

Browse files
sheetalkamatsheetalkamat
authored
Handle localness in special cases by checking exported variable assignment (#43851)
* Handle localness in special cases by checking exported variable assignment Fixes #42976 * Fix existing tests where arrow now behaves similar to function expression * Update src/services/goToDefinition.ts
1 parent c9eb62f commit c96b472

File tree

7 files changed

+348
-9
lines changed

7 files changed

+348
-9
lines changed

‎src/compiler/utilitiesPublic.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,7 @@ namespace ts {
619619
export function getNameOfDeclaration(declaration: Declaration | Expression | undefined): DeclarationName | undefined {
620620
if (declaration === undefined) return undefined;
621621
return getNonAssignedNameOfDeclaration(declaration) ||
622-
(isFunctionExpression(declaration) || isClassExpression(declaration) ? getAssignedName(declaration) : undefined);
622+
(isFunctionExpression(declaration) || isArrowFunction(declaration) || isClassExpression(declaration) ? getAssignedName(declaration) : undefined);
623623
}
624624

625625
/*@internal*/

‎src/services/goToDefinition.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,10 +326,41 @@ namespace ts.GoToDefinition {
326326
sourceFile,
327327
FindAllReferences.getContextNode(declaration)
328328
),
329-
isLocal: !checker.isDeclarationVisible(declaration)
329+
isLocal: !isDefinitionVisible(checker, declaration)
330330
};
331331
}
332332

333+
function isDefinitionVisible(checker: TypeChecker, declaration: Declaration): boolean {
334+
if (checker.isDeclarationVisible(declaration)) return true;
335+
if (!declaration.parent) return false;
336+
337+
// Variable initializers are visible if variable is visible
338+
if (hasInitializer(declaration.parent) && declaration.parent.initializer === declaration) return isDefinitionVisible(checker, declaration.parent as Declaration);
339+
340+
// Handle some exceptions here like arrow function, members of class and object literal expression which are technically not visible but we want the definition to be determined by its parent
341+
switch (declaration.kind) {
342+
case SyntaxKind.PropertyDeclaration:
343+
case SyntaxKind.GetAccessor:
344+
case SyntaxKind.SetAccessor:
345+
case SyntaxKind.MethodDeclaration:
346+
// Private/protected properties/methods are not visible
347+
if (hasEffectiveModifier(declaration, ModifierFlags.Private)) return false;
348+
// Public properties/methods are visible if its parents are visible, so:
349+
// falls through
350+
351+
case SyntaxKind.Constructor:
352+
case SyntaxKind.PropertyAssignment:
353+
case SyntaxKind.ShorthandPropertyAssignment:
354+
case SyntaxKind.ObjectLiteralExpression:
355+
case SyntaxKind.ClassExpression:
356+
case SyntaxKind.ArrowFunction:
357+
case SyntaxKind.FunctionExpression:
358+
return isDefinitionVisible(checker, declaration.parent as Declaration);
359+
default:
360+
return false;
361+
}
362+
}
363+
333364
function createDefinitionFromSignatureDeclaration(typeChecker: TypeChecker, decl: SignatureDeclaration): DefinitionInfo {
334365
return createDefinitionInfo(decl, typeChecker, decl.symbol, decl);
335366
}

‎src/testRunner/unittests/tsserver/projectReferences.ts

Lines changed: 299 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,305 @@ bar();
621621
checkProjectActualFiles(service.configuredProjects.get(servicesConfig.path)!, [servicesFile.path, servicesConfig.path, libFile.path, typesFile.path, programFile.path]);
622622
});
623623

624+
describe("special handling of localness of the definitions for findAllRefs", () => {
625+
function setup(definition: string, usage: string) {
626+
const solutionLocation = "/user/username/projects/solution";
627+
const solution: File = {
628+
path: `${solutionLocation}/tsconfig.json`,
629+
content: JSON.stringify({
630+
files: [],
631+
references: [
632+
{ path: "./api" },
633+
{ path: "./app" },
634+
]
635+
})
636+
};
637+
const apiConfig: File = {
638+
path: `${solutionLocation}/api/tsconfig.json`,
639+
content: JSON.stringify({
640+
compilerOptions: {
641+
composite: true,
642+
outDir: "dist",
643+
rootDir: "src",
644+
},
645+
include: ["src"],
646+
references: [{ path: "../shared" }]
647+
})
648+
};
649+
const apiFile: File = {
650+
path: `${solutionLocation}/api/src/server.ts`,
651+
content: `import * as shared from "../../shared/dist";
652+
${usage}`
653+
};
654+
const appConfig: File = {
655+
path: `${solutionLocation}/app/tsconfig.json`,
656+
content: apiConfig.content
657+
};
658+
const appFile: File = {
659+
path: `${solutionLocation}/app/src/app.ts`,
660+
content: apiFile.content
661+
};
662+
const sharedConfig: File = {
663+
path: `${solutionLocation}/shared/tsconfig.json`,
664+
content: JSON.stringify({
665+
compilerOptions: {
666+
composite: true,
667+
outDir: "dist",
668+
rootDir: "src",
669+
},
670+
include: ["src"]
671+
})
672+
};
673+
const sharedFile: File = {
674+
path: `${solutionLocation}/shared/src/index.ts`,
675+
content: definition
676+
};
677+
const host = createServerHost([libFile, solution, libFile, apiConfig, apiFile, appConfig, appFile, sharedConfig, sharedFile]);
678+
const session = createSession(host);
679+
const service = session.getProjectService();
680+
service.openClientFile(apiFile.path);
681+
verifyApiProjectLoadAndSolutionPending();
682+
return { session, verifySolutionTreeLoaded, verifyApiAndSharedProjectLoadAndSolutionPending, apiFile, appFile, sharedFile };
683+
684+
function checkApiProject() {
685+
const apiProject = service.configuredProjects.get(apiConfig.path)!;
686+
checkProjectActualFiles(apiProject, [libFile.path, apiConfig.path, apiFile.path, sharedFile.path]);
687+
}
688+
function checkAppProject() {
689+
const appProject = service.configuredProjects.get(appConfig.path)!;
690+
checkProjectActualFiles(appProject, [libFile.path, appConfig.path, appFile.path, sharedFile.path]);
691+
}
692+
function checkSharedProject() {
693+
const sharedProject = service.configuredProjects.get(sharedConfig.path)!;
694+
checkProjectActualFiles(sharedProject, [libFile.path, sharedConfig.path, sharedFile.path]);
695+
}
696+
function checkSolutionLoadPending() {
697+
const solutionProject = service.configuredProjects.get(solution.path)!;
698+
assert.isFalse(solutionProject.isInitialLoadPending());
699+
}
700+
function checkSolutionLoadComplete() {
701+
const solutionProject = service.configuredProjects.get(solution.path)!;
702+
assert.isTrue(solutionProject.isInitialLoadPending());
703+
}
704+
function verifySolutionTreeLoaded() {
705+
checkNumberOfProjects(service, { configuredProjects: 4 });
706+
checkApiProject();
707+
checkAppProject();
708+
checkSharedProject();
709+
checkSolutionLoadPending();
710+
}
711+
712+
function verifyApiProjectLoadAndSolutionPending() {
713+
checkNumberOfProjects(service, { configuredProjects: 2 });
714+
checkApiProject();
715+
checkSolutionLoadComplete();
716+
}
717+
718+
function verifyApiAndSharedProjectLoadAndSolutionPending() {
719+
checkNumberOfProjects(service, { configuredProjects: 3 });
720+
checkApiProject();
721+
checkSharedProject();
722+
checkSolutionLoadComplete();
723+
}
724+
}
725+
726+
it("when using arrow function assignment", () => {
727+
const { session, apiFile, appFile, sharedFile, verifySolutionTreeLoaded } = setup(
728+
`export const dog = () => { };`,
729+
`shared.dog();`
730+
);
731+
732+
// Find all references
733+
const response = session.executeCommandSeq<protocol.ReferencesRequest>({
734+
command: protocol.CommandTypes.References,
735+
arguments: protocolFileLocationFromSubstring(apiFile, "dog")
736+
}).response as protocol.ReferencesResponseBody;
737+
assert.deepEqual(response, {
738+
refs: [
739+
makeReferenceItem({
740+
file: sharedFile,
741+
text: "dog",
742+
contextText: sharedFile.content,
743+
isDefinition: true,
744+
lineText: sharedFile.content,
745+
}),
746+
makeReferenceItem({
747+
file: apiFile,
748+
text: "dog",
749+
isDefinition: false,
750+
lineText: "shared.dog();",
751+
}),
752+
makeReferenceItem({
753+
file: appFile,
754+
text: "dog",
755+
isDefinition: false,
756+
lineText: "shared.dog();",
757+
})
758+
],
759+
symbolName: "dog",
760+
symbolStartOffset: protocolLocationFromSubstring(apiFile.content, "dog").offset,
761+
symbolDisplayString: "const dog: () => void"
762+
});
763+
verifySolutionTreeLoaded();
764+
});
765+
766+
it("when using arrow function as object literal property", () => {
767+
const { session, apiFile, appFile, sharedFile, verifySolutionTreeLoaded } = setup(
768+
`export const foo = { bar: () => { } };`,
769+
`shared.foo.bar();`
770+
);
771+
772+
// Find all references
773+
const response = session.executeCommandSeq<protocol.ReferencesRequest>({
774+
command: protocol.CommandTypes.References,
775+
arguments: protocolFileLocationFromSubstring(apiFile, "bar")
776+
}).response as protocol.ReferencesResponseBody;
777+
assert.deepEqual(response, {
778+
refs: [
779+
makeReferenceItem({
780+
file: sharedFile,
781+
text: "bar",
782+
contextText: `bar: () => { }`,
783+
isDefinition: true,
784+
lineText: sharedFile.content,
785+
}),
786+
makeReferenceItem({
787+
file: apiFile,
788+
text: "bar",
789+
isDefinition: false,
790+
lineText: "shared.foo.bar();",
791+
}),
792+
makeReferenceItem({
793+
file: appFile,
794+
text: "bar",
795+
isDefinition: false,
796+
lineText: "shared.foo.bar();",
797+
})
798+
],
799+
symbolName: "bar",
800+
symbolStartOffset: protocolLocationFromSubstring(apiFile.content, "bar").offset,
801+
symbolDisplayString: "(property) bar: () => void"
802+
});
803+
verifySolutionTreeLoaded();
804+
});
805+
806+
it("when using object literal property", () => {
807+
const { session, apiFile, appFile, sharedFile, verifySolutionTreeLoaded } = setup(
808+
`export const foo = { baz: "BAZ" };`,
809+
`shared.foo.baz;`
810+
);
811+
812+
// Find all references
813+
const response = session.executeCommandSeq<protocol.ReferencesRequest>({
814+
command: protocol.CommandTypes.References,
815+
arguments: protocolFileLocationFromSubstring(apiFile, "baz")
816+
}).response as protocol.ReferencesResponseBody;
817+
assert.deepEqual(response, {
818+
refs: [
819+
makeReferenceItem({
820+
file: sharedFile,
821+
text: "baz",
822+
contextText: `baz: "BAZ"`,
823+
isDefinition: true,
824+
lineText: sharedFile.content,
825+
}),
826+
makeReferenceItem({
827+
file: apiFile,
828+
text: "baz",
829+
isDefinition: false,
830+
lineText: "shared.foo.baz;",
831+
}),
832+
makeReferenceItem({
833+
file: appFile,
834+
text: "baz",
835+
isDefinition: false,
836+
lineText: "shared.foo.baz;",
837+
})
838+
],
839+
symbolName: "baz",
840+
symbolStartOffset: protocolLocationFromSubstring(apiFile.content, "baz").offset,
841+
symbolDisplayString: `(property) baz: string`
842+
});
843+
verifySolutionTreeLoaded();
844+
});
845+
846+
it("when using method of class expression", () => {
847+
const { session, apiFile, appFile, sharedFile, verifySolutionTreeLoaded } = setup(
848+
`export const foo = class { fly() {} };`,
849+
`const instance = new shared.foo();
850+
instance.fly();`
851+
);
852+
853+
// Find all references
854+
const response = session.executeCommandSeq<protocol.ReferencesRequest>({
855+
command: protocol.CommandTypes.References,
856+
arguments: protocolFileLocationFromSubstring(apiFile, "fly")
857+
}).response as protocol.ReferencesResponseBody;
858+
assert.deepEqual(response, {
859+
refs: [
860+
makeReferenceItem({
861+
file: sharedFile,
862+
text: "fly",
863+
contextText: `fly() {}`,
864+
isDefinition: true,
865+
lineText: sharedFile.content,
866+
}),
867+
makeReferenceItem({
868+
file: apiFile,
869+
text: "fly",
870+
isDefinition: false,
871+
lineText: "instance.fly();",
872+
}),
873+
makeReferenceItem({
874+
file: appFile,
875+
text: "fly",
876+
isDefinition: false,
877+
lineText: "instance.fly();",
878+
})
879+
],
880+
symbolName: "fly",
881+
symbolStartOffset: protocolLocationFromSubstring(apiFile.content, "fly").offset,
882+
symbolDisplayString: `(method) foo.fly(): void`
883+
});
884+
verifySolutionTreeLoaded();
885+
});
886+
887+
it("when using arrow function as object literal property is loaded through indirect assignment with original declaration local to project is treated as local", () => {
888+
const { session, apiFile, sharedFile, verifyApiAndSharedProjectLoadAndSolutionPending } = setup(
889+
`const local = { bar: () => { } };
890+
export const foo = local;`,
891+
`shared.foo.bar();`
892+
);
893+
894+
// Find all references
895+
const response = session.executeCommandSeq<protocol.ReferencesRequest>({
896+
command: protocol.CommandTypes.References,
897+
arguments: protocolFileLocationFromSubstring(apiFile, "bar")
898+
}).response as protocol.ReferencesResponseBody;
899+
assert.deepEqual(response, {
900+
refs: [
901+
makeReferenceItem({
902+
file: sharedFile,
903+
text: "bar",
904+
contextText: `bar: () => { }`,
905+
isDefinition: true,
906+
lineText: `const local = { bar: () => { } };`,
907+
}),
908+
makeReferenceItem({
909+
file: apiFile,
910+
text: "bar",
911+
isDefinition: false,
912+
lineText: "shared.foo.bar();",
913+
}),
914+
],
915+
symbolName: "bar",
916+
symbolStartOffset: protocolLocationFromSubstring(apiFile.content, "bar").offset,
917+
symbolDisplayString: "(property) bar: () => void"
918+
});
919+
verifyApiAndSharedProjectLoadAndSolutionPending();
920+
});
921+
});
922+
624923
it("when disableSolutionSearching is true, solution and siblings are not loaded", () => {
625924
const solutionLocation = "/user/username/projects/solution";
626925
const solution: File = {

0 commit comments

Comments
 (0)