8000 Syntax only server creates inferred project with all the open files w… by sheetalkamat · Pull Request #38561 · microsoft/TypeScript · GitHub
[go: up one dir, main page]

Skip to content
8000

Syntax only server creates inferred project with all the open files w… #38561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

8000
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/compiler/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,13 +234,14 @@ namespace ts {
}

export const noopFileWatcher: FileWatcher = { close: noop };
export const returnNoopFileWatcher = () => noopFileWatcher;

export function createWatchHost(system = sys, reportWatchStatus?: WatchStatusReporter): WatchHost {
const onWatchStatusChange = reportWatchStatus || createWatchStatusReporter(system);
return {
onWatchStatusChange,
watchFile: maybeBind(system, system.watchFile) || (() => noopFileWatcher),
watchDirectory: maybeBind(system, system.watchDirectory) || (() => noopFileWatcher),
watchFile: maybeBind(system, system.watchFile) || returnNoopFileWatcher,
watchDirectory: maybeBind(system, system.watchDirectory) || returnNoopFileWatcher,
setTimeout: maybeBind(system, system.setTimeout) || noop,
clearTimeout: maybeBind(system, system.clearTimeout) || noop
};
Expand Down
8 changes: 7 additions & 1 deletion src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,13 @@ namespace ts.server {
const watchLogLevel = this.logger.hasLevel(LogLevel.verbose) ? WatchLogLevel.Verbose :
this.logger.loggingEnabled() ? WatchLogLevel.TriggerOnly : WatchLogLevel.None;
const log: (s: string) => void = watchLogLevel !== WatchLogLevel.None ? (s => this.logger.info(s)) : noop;
this.watchFactory = getWatchFactory(watchLogLevel, log, getDetailWatchInfo);
this.watchFactory = this.syntaxOnly ?
{
watchFile: returnNoopFileWatcher,
watchFilePath: returnNoopFileWatcher,
watchDirectory: returnNoopFileWatcher,
} :
getWatchFactory(watchLogLevel, log, getDetailWatchInfo);
}

toPath(fileName: string) {
Expand Down
14 changes: 8 additions & 6 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,10 @@ namespace ts.server {
this.compilerOptions.allowNonTsExtensions = true;
}

this.languageServiceEnabled = !projectService.syntaxOnly;
this.languageServiceEnabled = true;
if (projectService.syntaxOnly) {
this.compilerOptions.noResolve = true;
}

this.setInternalCompilerOptionsForEmittingJsFiles();
const host = this.projectService.host;
Expand All @@ -293,7 +296,7 @@ namespace ts.server {

// Use the current directory as resolution root only if the project created using current directory string
this.resolutionCache = createResolutionCache(this, currentDirectory && this.currentDirectory, /*logChangesWhenResolvingModule*/ true);
this.languageService = createLanguageService(this, this.documentRegistry, projectService.syntaxOnly);
this.languageService = createLanguageService(this, this.documentRegistry, this.projectService.syntaxOnly);
if (lastFileExceededProgramSize) {
this.disableLanguageService(lastFileExceededProgramSize);
}
Expand Down Expand Up @@ -642,7 +645,7 @@ namespace ts.server {
}

enableLanguageService() {
if (this.languageServiceEnabled || this.projectService.syntaxOnly) {
if (this.languageServiceEnabled) {
return;
}
this.languageServiceEnabled = true;
Expand All @@ -654,7 +657,6 @@ namespace ts.server {
if (!this.languageServiceEnabled) {
return;
}
Debug.assert(!this.projectService.syntaxOnly);
this.languageService.cleanupSemanticCache();
this.languageServiceEnabled = false;
this.lastFileExceededProgramSize = lastFileExceededProgramSize;
Expand Down Expand Up @@ -970,7 +972,7 @@ namespace ts.server {

// update builder only if language service is enabled
// otherwise tell it to drop its internal state
if (this.languageServiceEnabled) {
if (this.languageServiceEnabled && !this.projectService.syntaxOnly) {
// 1. no changes in structure, no changes in unresolved imports - do nothing
// 2. no changes in structure, unresolved imports were changed - collect unresolved imports for all files
// (can reuse cached imports for files that were not changed)
Expand Down Expand Up @@ -1092,7 +1094,7 @@ namespace ts.server {
}

// Watch the type locations that would be added to program as part of automatic type resolutions
if (this.languageServiceEnabled) {
if (this.languageServiceEnabled && !this.projectService.syntaxOnly) {
this.resolutionCache.updateTypeRootsWatch();
}
}
Expand Down
49 changes: 47 additions & 2 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,42 @@ namespace ts.server {
undefined;
}

const invalidSyntaxOnlyCommands: readonly CommandNames[] = [
CommandNames.OpenExternalProject,
CommandNames.OpenExternalProjects,
CommandNames.CloseExternalProject,
CommandNames.SynchronizeProjectList,
CommandNames.EmitOutput,
CommandNames.CompileOnSaveAffectedFileList,
CommandNames.CompileOnSaveEmitFile,
CommandNames.CompilerOptionsDiagnosticsFull,
CommandNames.EncodedSemanticClassificationsFull,
CommandNames.SemanticDiagnosticsSync,
Copy link
Member Author

Choose a reason for hiding this comment

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

SyntacticDiagnosticsSync: This is somewhat project setting dependent ? Atleast at some point parsing errors use to be different based on target .. eg target determines what unicode is considered identifier start... So i am not sure if this should be enabled.. But if we do enable we i was wondering if GetErr should only do syntax checks and skip semantic and suggetion diagnostics on syntax server

Some questionable which i have disabled for now
Reload
ReloadProjects
PrepareCallHierarchy
ProvideCallHierarchyIncomingCalls
ProvideCallHierarchyOutgoingCalls

CommandNames.SyntacticDiagnosticsSync,
CommandNames.SuggestionDiagnosticsSync,
CommandNames.Geterr,
CommandNames.GeterrForProject,
CommandNames.Reload,
CommandNames.ReloadProjects,
CommandNames.GetCodeFixes,
CommandNames.GetCodeFixesFull,
CommandNames.GetCombinedCodeFix,
CommandNames.GetCombinedCodeFixFull,
CommandNames.ApplyCodeActionCommand,
CommandNames.GetSupportedCodeFixes,
CommandNames.GetApplicableRefactors,
CommandNames.GetEditsForRefactor,
CommandNames.GetEditsForRefactorFull,
CommandNames.OrganizeImports,
CommandNames.OrganizeImportsFull,
CommandNames.GetEditsForFileRename,
CommandNames.GetEditsForFileRenameFull,
CommandNames.ConfigurePlugin,
CommandNames.PrepareCallHierarchy,
CommandNames.ProvideCallHierarchyIncomingCalls,
CommandNames.ProvideCallHierarchyOutgoingCalls,
];

export interface SessionOptions {
host: ServerHost;
cancellationToken: ServerCancellationToken;
Expand Down Expand Up @@ -667,6 +703,15 @@ namespace ts.server {
this.projectService = new ProjectService(settings);
this.projectService.setPerformanceEventHandler(this.performanceEventHandler.bind(this));
this.gcTimer = new GcTimer(this.host, /*delay*/ 7000, this.logger);

// Make sure to setup handlers to throw error for not allowed commands on syntax server;
if (this.projectService.syntaxOnly) {
invalidSyntaxOnlyCommands.forEach(commandName =>
this.handlers.set(commandName, request => {
throw new Error(`Request: ${request.command} not allowed on syntaxServer`);
})
);
}
}

private sendRequestCompletedEvent(requestId: number): void {
Expand Down Expand Up @@ -1253,9 +1298,9 @@ namespace ts.server {
}

private getJsxClosingTag(args: protocol.JsxClosingTagRequestArgs): TextInsertion | undefined {
const { file, project } = this.getFileAndProject(args);
const { file, languageService } = this.getFileAndLanguageServiceForSyntacticOperation(args);
const position = this.getPositionInFile(args, file);
const tag = project.getLanguageService().getJsxClosingTagAtPosition(file, position);
const tag = languageService.getJsxClosingTagAtPosition(file, position);
return tag === undefined ? undefined : { newText: tag.newText, caretOffset: 0 };
}

Expand Down
39 changes: 31 additions & 8 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,26 @@ namespace ts {
}
}

const invalidOperationsOnSyntaxOnly: readonly (keyof LanguageService)[] = [
Copy link
Member

Choose a reason for hiding this comment

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

How does this relate to the list in session.ts? Do they need to stay in sync? Is one redundant?

"getSyntacticDiagnostics",
"getSemanticDiagnostics",
"getSuggestionDiagnostics",
"getCompilerOptionsDiagnostics",
"getSemanticClassifications",
"getEncodedSemanticClassifications",
"getCodeFixesAtPosition",
"getCombinedCodeFix",
"applyCodeActionCommand",
"organizeImports",
"getEditsForFileRename",
"getEmitOutput",
"getApplicableRefactors",
"getEditsForRefactor",
"prepareCallHierarchy",
"provideCallHierarchyIncomingCalls",
"provideCallHierarchyOutgoingCalls",
];

export function createLanguageService(
host: LanguageServiceHost,
documentRegistry: DocumentRegistry = createDocumentRegistry(host.useCaseSensitiveFileNames && host.useCaseSensitiveFileNames(), host.getCurrentDirectory()),
Expand Down Expand Up @@ -1224,8 +1244,6 @@ namespace ts {
}

function synchronizeHostData(): void {
Debug.assert(!syntaxOnly);

// perform fast check if host supports it
if (host.getProjectVersion) {
const hostProjectVersion = host.getProjectVersion();
Expand Down Expand Up @@ -1419,11 +1437,6 @@ namespace ts {

// TODO: GH#18217 frequently asserted as defined
function getProgram(): Program | undefined {
if (syntaxOnly) {
Debug.assert(program === undefined);
return undefined;
}

synchronizeHostData();

return program;
Expand Down Expand Up @@ -2199,7 +2212,7 @@ namespace ts {
return declaration ? CallHierarchy.getOutgoingCalls(program, declaration) : [];
}

return {
const ls: LanguageService = {
dispose,
cleanupSemanticCache,
getSyntacticDiagnostics,
Expand Down Expand Up @@ -2259,6 +2272,16 @@ namespace ts {
provideCallHierarchyIncomingCalls,
provideCallHierarchyOutgoingCalls
};

if (syntaxOnly) {
invalidOperationsOnSyntaxOnly.forEach(key =>
ls[key] = () => {
throw new Error(`LanguageService Operation: ${key} not allowed on syntaxServer`);
}
);
}

return ls;
}

/* @internal */
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@
"unittests/tsserver/reload.ts",
"unittests/tsserver/rename.ts",
"unittests/tsserver/resolutionCache.ts",
"unittests/tsserver/semanticOperationsOnSyntaxServer.ts",
"unittests/tsserver/smartSelection.ts",
"unittests/tsserver/session.ts",
"unittests/tsserver/skipLibCheck.ts",
Expand Down
2 changes: 1 addition & 1 deletion src/testRunner/unittests/tsserver/inferredProjects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ namespace ts.projectSystem {
const proj = projectService.inferredProjects[0];
assert.isDefined(proj);

assert.isFalse(proj.languageServiceEnabled);
assert.isTrue(proj.languageServiceEnabled);
});

it("project settings for inferred projects", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
namespace ts.projectSystem {
describe("unittests:: tsserver:: Semantic operations on Syntax server", () => {
function setup() {
const file1: File = {
path: `${tscWatch.projectRoot}/a.ts`,
content: `import { y } from "./b";
class c { prop = "hello"; foo() { return this.prop; } }`
};
const file2: File = {
path: `${tscWatch.projectRoot}/b.ts`,
content: "export const y = 10;"
};
const configFile: File = {
path: `${tscWatch.projectRoot}/tsconfig.json`,
content: "{}"
};
const host = createServerHost([file1, file2, libFile, configFile]);
const session = createSession(host, { syntaxOnly: true, useSingleInferredProject: true });
return { host, session, file1, file2, configFile };
}

it("open files are added to inferred project even if config file is present and semantic operations succeed", () => {
const { host, session, file1, file2 } = setup();
const service = session.getProjectService();
openFilesForSession([file1], session);
checkNumberOfProjects(service, { inferredProjects: 1 });
const project = service.inferredProjects[0];
checkProjectActualFiles(project, [libFile.path, file1.path]); // Import is not resolved
verifyCompletions();

openFilesForSession([file2], session);
checkNumberOfProjects(service, { inferredProjects: 1 });
checkProjectActualFiles(project, [libFile.path, file1.path, file2.path]);
verifyCompletions();

function verifyCompletions() {
assert.isTrue(project.languageServiceEnabled);
checkWatchedFiles(host, emptyArray);
checkWatchedDirectories(host, emptyArray, /*recursive*/ true);
checkWatchedDirectories(host, emptyArray, /*recursive*/ false);
const response = session.executeCommandSeq<protocol.CompletionsRequest>({
command: protocol.CommandTypes.Completions,
arguments: protocolFileLocationFromSubstring(file1, "prop", { index: 1 })
}).response as protocol.CompletionEntry[];
assert.deepEqual(response, [
completionEntry("foo", ScriptElementKind.memberFunctionElement),
completionEntry("prop", ScriptElementKind.memberVariableElement),
]);
}

function completionEntry(name: string, kind: ScriptElementKind): protocol.CompletionEntry {
return {
name,
kind,
kindModifiers: "",
s 9583 ortText: Completions.SortText.LocationPriority,
hasAction: undefined,
insertText: undefined,
isRecommended: undefined,
replacementSpan: undefined,
source: undefined
};
}
});

it("throws on unsupported commands", () => {
const { session, file1 } = setup();
const service = session.getProjectService();
openFilesForSession([file1], session);
let hasException = false;
const request: protocol.SemanticDiagnosticsSyncRequest = {
type: "request",
seq: 1,
command: protocol.CommandTypes.SemanticDiagnosticsSync,
arguments: { file: file1.path }
};
try {
session.executeCommand(request);
}
catch (e) {
assert.equal(e.message, `Request: semanticDiagnosticsSync not allowed on syntaxServer`);
hasException = true;
}
assert.isTrue(hasException);

hasException = false;
const project = service.inferredProjects[0];
try {
project.getLanguageService().getSemanticDiagnostics(file1.path);
}
catch (e) {
assert.equal(e.message, `LanguageService Operation: getSemanticDiagnostics not allowed on syntaxServer`);
hasException = true;
}
assert.isTrue(hasException);
});
});
}
0