Skip to content

Commit

Permalink
Handle changes to inferred projects during FAR
Browse files Browse the repository at this point in the history
In some scenarios (see the new test for an example), FAR may search an inferred project and then subsequently trigger the loading of a configured project containing (some of) the same files.  When the configured project is loaded, the files are removed from the inferred project and added to the configured project, potentially _invalidating the results already computed for the inferred project_.  This change introduces a cleanup pass that removes results from `dirty` projects if the corresponding files are no longer in the project.

Caught by microsoft#50679
  • Loading branch information
amcasey committed Sep 16, 2022
1 parent 4110b80 commit 22de0db
Show file tree
Hide file tree
Showing 3 changed files with 256 additions and 3 deletions.
65 changes: 62 additions & 3 deletions src/server/session.ts
Expand Up @@ -300,7 +300,7 @@ namespace ts.server {
};

function createDocumentSpanSet(): Set<DocumentSpan> {
return createSet(({textSpan}) => textSpan.start + 100003 * textSpan.length, documentSpansEqual);
return createSet(({ textSpan }) => textSpan.start + 100003 * textSpan.length, documentSpansEqual);
}

function getRenameLocationsWorker(
Expand Down Expand Up @@ -377,6 +377,65 @@ namespace ts.server {
return perProjectResults;
}

// Corner case: since FAR can cause projects to be loaded, it can cause files to be removed
// from inferred projects, possibly invalidating the results for those projects.

// NB: Iterate over a copy since we're going to modify the original as we go
for (const [project, referencedSymbols] of arrayFrom(perProjectResults.entries())) {
if (!project.dirty) {
continue;
}

const program = project.getLanguageService().getProgram()!;

let newReferencedSymbols: ReferencedSymbol[] | undefined;

for (let i = 0; i < referencedSymbols.length; i++) {
const referencedSymbol = referencedSymbols[i];
const def = referencedSymbol.definition;
if (!program.getSourceFile(def.fileName)) {
// If the def file is no longer in the project, the ref files must not be either -
// otherwise, they'd pull the ref file in
if (!newReferencedSymbols) {
newReferencedSymbols = referencedSymbols.slice(0, i); // i.e. Don't include the current element
}
continue;
}

const references = referencedSymbol.references;
let newReferences: ReferencedSymbolEntry[] | undefined;
for (let j = 0; j < references.length; j++) {
const ref = references[j];
if (!program.getSourceFile(ref.fileName)) {
// If the ref file is no longer in the project, it must have pulled the def into
// its new project, so there's no risk of losing information by dropping it
if (!newReferences) {
newReferences = references.slice(0, j); // i.e. Don't include the current element
}
continue;
}
}

if (newReferences) {
if (!newReferencedSymbols) {
newReferencedSymbols = referencedSymbols.slice(0, i); // i.e. Don't include the current element
}

// Don't bother to add it if there are no remaining references
if (newReferences.length) {
newReferencedSymbols.push({ definition: def, references: newReferences });
}
}
else if (newReferencedSymbols) {
newReferencedSymbols.push(referencedSymbol);
}
}

if (newReferencedSymbols) {
perProjectResults.set(project, newReferencedSymbols);
}
}

// `isDefinition` is only (definitely) correct in `defaultProject` because we might
// have started the other project searches from related symbols. Propagate the
// correct results to all other projects.
Expand Down Expand Up @@ -2589,7 +2648,7 @@ namespace ts.server {
try {
codeActions = project.getLanguageService().getCodeFixesAtPosition(file, startPosition, endPosition, args.errorCodes, this.getFormatOptions(file), this.getPreferences(file));
}
catch(e) {
catch (e) {
const ls = project.getLanguageService();
const existingDiagCodes = [
...ls.getSyntacticDiagnostics(file),
Expand Down Expand Up @@ -3537,6 +3596,6 @@ namespace ts.server {
&& typeof data.exportName === "string"
&& (data.fileName === undefined || typeof data.fileName === "string")
&& (data.ambientModuleName === undefined || typeof data.ambientModuleName === "string"
&& (data.isPackageJsonImport === undefined || typeof data.isPackageJsonImport === "boolean"));
&& (data.isPackageJsonImport === undefined || typeof data.isPackageJsonImport === "boolean"));
}
}
166 changes: 166 additions & 0 deletions tests/baselines/reference/referencesInInferredProject.baseline.jsonc
@@ -0,0 +1,166 @@
// === /proj/__test__/a.test.ts ===
// import { [|x|] } from "../a";
// [|x|].toString();

// === /proj/a.ts ===
// export const /*FIND ALL REFS*/[|x|] = 1;

[
{
"definition": {
"containerKind": "",
"containerName": "",
"fileName": "/proj/a.ts",
"kind": "const",
"name": "const x: 1",
"textSpan": {
"start": 13,
"length": 1
},
"displayParts": [
{
"text": "const",
"kind": "keyword"
},
{
"text": " ",
"kind": "space"
},
{
"text": "x",
"kind": "localName"
},
{
"text": ":",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "1",
"kind": "stringLiteral"
}
],
"contextSpan": {
"start": 0,
"length": 19
}
},
"references": [
{
"textSpan": {
"start": 13,
"length": 1
},
"fileName": "/proj/a.ts",
"contextSpan": {
"start": 0,
"length": 19
},
"isWriteAccess": true,
"isDefinition": true
}
]
},
{
"definition": {
"containerKind": "",
"containerName": "",
"fileName": "/proj/__test__/a.test.ts",
"kind": "alias",
"name": "(alias) const x: 1\nimport x",
"textSpan": {
"start": 9,
"length": 1
},
"displayParts": [
{
"text": "(",
"kind": "punctuation"
},
{
"text": "alias",
"kind": "text"
},
{
"text": ")",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "const",
"kind": "keyword"
},
{
"text": " ",
"kind": "space"
},
{
"text": "x",
"kind": "aliasName"
},
{
"text": ":",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "1",
"kind": "stringLiteral"
},
{
"text": "\n",
"kind": "lineBreak"
},
{
"text": "import",
"kind": "keyword"
},
{
"text": " ",
"kind": "space"
},
{
"text": "x",
"kind": "aliasName"
}
],
"contextSpan": {
"start": 0,
"length": 25
}
},
"references": [
{
"textSpan": {
"start": 9,
"length": 1
},
"fileName": "/proj/__test__/a.test.ts",
"contextSpan": {
"start": 0,
"length": 25
},
"isWriteAccess": true,
"isDefinition": false
},
{
"textSpan": {
"start": 26,
"length": 1
},
"fileName": "/proj/__test__/a.test.ts",
"isWriteAccess": false,
"isDefinition": false
}
]
}
]
28 changes: 28 additions & 0 deletions tests/cases/fourslash/server/referencesInInferredProject.ts
@@ -0,0 +1,28 @@
/// <reference path="../fourslash.ts"/>

// @Filename: /tsconfig.json
////{ }

// @Filename: /proj/tsconfig.json
////{
//// "compilerOptions": {
//// "composite": true,
//// },
//// "exclude": ["**/__test__/"]
////}

// @Filename: /proj/a.ts
////export const /*1*/x = 1;

// @Filename: /proj/__test__/a.test.ts
////import { x } from "../a";
////x.toString();

// Open both source files
// The test file will be in an inferred project, since its default project excludes it
// The product file will be in both its default configured project and the inferred project for the test file
goTo.file("/proj/a.ts");
goTo.file("/proj/__test__/a.test.ts");
// FAR will cause the root tsconfig to be loaded (since the LS happens to know about it and it could contain references),
// which will *remove* both files from the inferred project, invalidating any references attributed to that project
verify.baselineFindAllReferences('1')

0 comments on commit 22de0db

Please sign in to comment.