refactor

Author: gabritto

src/server/session.ts

@@ -994,7 +994,8 @@ export class Session<TMessage = string> implements EventSender {
     private eventHandler: ProjectServiceEventHandler | undefined;
     private readonly noGetErrOnBackgroundUpdate?: boolean;
 
-    private diagnosticsTime: [number, number] | undefined;
+    // Minimum number of lines for attempting to use region diagnostics for a file.
+    protected regionDiagLineCountThreshold = 500;
 
     constructor(opts: SessionOptions) {
         this.host = opts.host;
@@ -1260,53 +1261,61 @@ export class Session<TMessage = string> implements EventSender {
     }
 
     private semanticCheck(file: NormalizedPath, project: Project) {
-        this.diagnosticsTime = this.hrtime();
+        const diagnosticsStartTime = this.hrtime();
         tracing?.push(tracing.Phase.Session, "semanticCheck", { file, configFilePath: (project as ConfiguredProject).canonicalConfigFilePath }); // undefined is fine if the cast fails
         const diags = isDeclarationFileInJSOnlyNonConfiguredProject(project, file)
             ? emptyArray
             : project.getLanguageService().getSemanticDiagnostics(file).filter(d => !!d.file);
-        this.sendDiagnosticsEvent(file, project, diags, "semanticDiag");
+        this.sendDiagnosticsEvent(file, project, diags, "semanticDiag", diagnosticsStartTime);
         tracing?.pop();
     }
 
     private syntacticCheck(file: NormalizedPath, project: Project) {
-        this.diagnosticsTime = this.hrtime();
+        const diagnosticsStartTime = this.hrtime();
         tracing?.push(tracing.Phase.Session, "syntacticCheck", { file, configFilePath: (project as ConfiguredProject).canonicalConfigFilePath }); // undefined is fine if the cast fails
-        this.sendDiagnosticsEvent(file, project, project.getLanguageService().getSyntacticDiagnostics(file), "syntaxDiag");
+        this.sendDiagnosticsEvent(file, project, project.getLanguageService().getSyntacticDiagnostics(file), "syntaxDiag", diagnosticsStartTime);
         tracing?.pop();
     }
 
     private suggestionCheck(file: NormalizedPath, project: Project) {
-        this.diagnosticsTime = this.hrtime();
+        const diagnosticsStartTime = this.hrtime();
         tracing?.push(tracing.Phase.Session, "suggestionCheck", { file, configFilePath: (project as ConfiguredProject).canonicalConfigFilePath }); // undefined is fine if the cast fails
-        this.sendDiagnosticsEvent(file, project, project.getLanguageService().getSuggestionDiagnostics(file), "suggestionDiag");
+        this.sendDiagnosticsEvent(file, project, project.getLanguageService().getSuggestionDiagnostics(file), "suggestionDiag", diagnosticsStartTime);
         tracing?.pop();
     }
 
     private regionSemanticCheck(file: NormalizedPath, project: Project, ranges: TextRange[]): void {
-        this.diagnosticsTime = this.hrtime();
+        const diagnosticsStartTime = this.hrtime();
         tracing?.push(tracing.Phase.Session, "regionSemanticCheck", { file, configFilePath: (project as ConfiguredProject).canonicalConfigFilePath }); // undefined is fine if the cast fails
         let diagnosticsResult;
         if (!this.shouldDoRegionCheck(file) || !(diagnosticsResult = project.getLanguageService().getRegionSemanticDiagnostics(file, ranges))) {
             tracing?.pop();
             return;
         }
-        this.sendDiagnosticsEvent(file, project, diagnosticsResult.diagnostics, "regionSemanticDiag", diagnosticsResult.spans);
+        this.sendDiagnosticsEvent(file, project, diagnosticsResult.diagnostics, "regionSemanticDiag", diagnosticsStartTime, diagnosticsResult.spans);
         tracing?.pop();
         return;
     }
 
-    // We should only do the region-based semantic check if we think it would be considerably faster than a whole-file semantic check
+    // We should only do the region-based semantic check if we think it would be
+    // considerably faster than a whole-file semantic check.
     /** @internal */
     protected shouldDoRegionCheck(file: NormalizedPath): boolean {
         const lineCount = this.projectService.getScriptInfoForNormalizedPath(file)?.textStorage.getLineInfo().getLineCount();
-        return !!(lineCount && lineCount > 500);
+        return !!(lineCount && lineCount >= this.regionDiagLineCountThreshold);
     }
 
-    private sendDiagnosticsEvent(file: NormalizedPath, project: Project, diagnostics: readonly Diagnostic[], kind: protocol.DiagnosticEventKind, spans?: TextSpan[]): void {
+    private sendDiagnosticsEvent(
+        file: NormalizedPath,
+        project: Project,
+        diagnostics: readonly Diagnostic[],
+        kind: protocol.DiagnosticEventKind,
+        diagnosticsStartTime: [number, number],
+        spans?: TextSpan[],
+    ): void {
         try {
             const scriptInfo = Debug.checkDefined(project.getScriptInfo(file));
-            const duration = hrTimeToMilliseconds(this.hrtime(this.diagnosticsTime));
+            const duration = hrTimeToMilliseconds(this.hrtime(diagnosticsStartTime));
 
             const body: protocol.DiagnosticEventBody = {
                 file,

src/testRunner/unittests/helpers/tsserver.ts

@@ -72,7 +72,7 @@ export interface TestSessionOptions extends ts.server.SessionOptions, TestTyping
     logger: LoggerWithInMemoryLogs;
     disableAutomaticTypingAcquisition?: boolean;
     useCancellationToken?: boolean | number;
-    alwaysDoRegionDiagnostics?: boolean;
+    regionDiagLineCountThreshold?: number;
 }
 export type TestSessionPartialOptionsAndHost = Partial<Omit<TestSessionOptions, "typingsInstaller" | "cancellationToken">> & Pick<TestSessionOptions, "host">;
 export type TestSessionConstructorOptions = TestServerHost | TestSessionPartialOptionsAndHost;
@@ -90,7 +90,6 @@ export class TestSession extends ts.server.Session {
     public override logger!: LoggerWithInMemoryLogs;
     public override readonly typingsInstaller!: TestTypingsInstallerAdapter;
     public serverCancellationToken: TestServerCancellationToken;
-    private alwaysDoRegionDiagnostics: boolean;
 
     constructor(optsOrHost: TestSessionConstructorOptions) {
         const opts = getTestSessionPartialOptionsAndHost(optsOrHost);
@@ -123,7 +122,9 @@ export class TestSession extends ts.server.Session {
             this,
             this.logger,
         );
-        this.alwaysDoRegionDiagnostics = !!opts.alwaysDoRegionDiagnostics;
+        if (opts.regionDiagLineCountThreshold !== undefined) {
+            this.regionDiagLineCountThreshold = opts.regionDiagLineCountThreshold;
+        }
     }
 
     getProjectService() {
@@ -158,10 +159,6 @@ export class TestSession extends ts.server.Session {
         request.type = "request";
         return this.executeCommand(request);
     }
-
-    protected override shouldDoRegionCheck(file: ts.server.NormalizedPath): boolean {
-        return this.alwaysDoRegionDiagnostics || super.shouldDoRegionCheck(file);
-    }
 }
 
 export function createSessionWithCustomEventHandler(

src/testRunner/unittests/tsserver/regionDiagnostics.ts

@@ -27,7 +27,7 @@ describe("unittests:: tsserver:: regionDiagnostics", () => {
                 foo(10, 50);`,
         };
         const host = createServerHost([file1, libFile]);
-        const session = new TestSession({ host, alwaysDoRegionDiagnostics: true });
+        const session = new TestSession({ host, regionDiagLineCountThreshold: 0 });
 
         openFilesForSession([file1], session);
 
@@ -89,7 +89,7 @@ describe("unittests:: tsserver:: regionDiagnostics", () => {
 
         const files = [file1, file2, file3, file4];
         const host = createServerHost([...files, libFile]);
-        const session = new TestSession({ host, alwaysDoRegionDiagnostics: true });
+        const session = new TestSession({ host, regionDiagLineCountThreshold: 0 });
 
         openFilesForSession(files, session);
 
@@ -148,7 +148,7 @@ describe("unittests:: tsserver:: regionDiagnostics", () => {
 
         it("region has suggestion diagnostics", () => {
             const host = createServerHost([config, indexFile, otherFile, libFile]);
-            const session = new TestSession({ host, alwaysDoRegionDiagnostics: true });
+            const session = new TestSession({ host, regionDiagLineCountThreshold: 0 });
 
             openFilesForSession([indexFile], session);
 
@@ -177,7 +177,7 @@ describe("unittests:: tsserver:: regionDiagnostics", () => {
 
         it("region does not have suggestion diagnostics", () => {
             const host = createServerHost([config, indexFile, otherFile, libFile]);
-            const session = new TestSession({ host, alwaysDoRegionDiagnostics: true });
+            const session = new TestSession({ host, regionDiagLineCountThreshold: 0 });
 
             openFilesForSession([indexFile], session);
 
@@ -218,7 +218,7 @@ describe("unittests:: tsserver:: regionDiagnostics", () => {
                 foo(10, 50);`,
         };
         const host = createServerHost([file1, libFile]);
-        const session = new TestSession({ host, alwaysDoRegionDiagnostics: false });
+        const session = new TestSession({ host });
 
         openFilesForSession([file1], session);
 
@@ -250,7 +250,7 @@ describe("unittests:: tsserver:: regionDiagnostics", () => {
                 foo(10, 50);`,
         };
         const host = createServerHost([file1, libFile]);
-        const session = new TestSession({ host, alwaysDoRegionDiagnostics: true });
+        const session = new TestSession({ host, regionDiagLineCountThreshold: 0 });
 
         openFilesForSession([file1], session);
 

tests/baselines/reference/api/typescript.d.ts

@@ -3455,7 +3455,7 @@ declare namespace ts {
             private suppressDiagnosticEvents?;
             private eventHandler;
             private readonly noGetErrOnBackgroundUpdate?;
-            private diagnosticsTime;
+            protected regionDiagLineCountThreshold: number;
             constructor(opts: SessionOptions);
             private sendRequestCompletedEvent;
             private addPerformanceData;