Merge #3614

3614: Separate persistent mutable state from config r=matklad a=matklad

That way, we clearly see which things are not change, and we also
clearly see which things are persistent.

r? @Veetaha 

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

editors/code/src/commands/server_version.ts

@@ -5,7 +5,7 @@ import { spawnSync } from 'child_process';
 
 export function serverVersion(ctx: Ctx): Cmd {
     return async () => {
-        const binaryPath = await ensureServerBinary(ctx.config);
+        const binaryPath = await ensureServerBinary(ctx.config, ctx.state);
 
         if (binaryPath == null) {
             throw new Error(

editors/code/src/config.ts

@@ -182,13 +182,6 @@ export class Config {
         return this.createGithubReleaseSource("rust-analyzer.vsix", NIGHTLY_TAG);
     }
 
-    readonly installedNightlyExtensionReleaseDate = new DateStorage(
-        "installed-nightly-extension-release-date",
-        this.ctx.globalState
-    );
-    readonly serverReleaseDate = new DateStorage("server-release-date", this.ctx.globalState);
-    readonly serverReleaseTag = new Storage<null | string>("server-release-tag", this.ctx.globalState, null);
-
     // We don't do runtime config validation here for simplicity. More on stackoverflow:
     // https://stackoverflow.com/questions/60135780/what-is-the-best-way-to-type-check-the-configuration-for-vscode-extension
 
@@ -232,37 +225,3 @@ export class Config {
     // for internal use
     get withSysroot() { return this.cfg.get("withSysroot", true) as boolean; }
 }
-
-export class Storage<T> {
-    constructor(
-        private readonly key: string,
-        private readonly storage: vscode.Memento,
-        private readonly defaultVal: T
-    ) { }
-
-    get(): T {
-        const val = this.storage.get(this.key, this.defaultVal);
-        log.debug(this.key, "==", val);
-        return val;
-    }
-    async set(val: T) {
-        log.debug(this.key, "=", val);
-        await this.storage.update(this.key, val);
-    }
-}
-export class DateStorage {
-    inner: Storage<null | string>;
-
-    constructor(key: string, storage: vscode.Memento) {
-        this.inner = new Storage(key, storage, null);
-    }
-
-    get(): null | Date {
-        const dateStr = this.inner.get();
-        return dateStr ? new Date(dateStr) : null;
-    }
-
-    async set(date: null | Date) {
-        await this.inner.set(date ? date.toString() : null);
-    }
-}

editors/code/src/ctx.ts

@@ -4,19 +4,21 @@ import * as lc from 'vscode-languageclient';
 import { Config } from './config';
 import { createClient } from './client';
 import { isRustEditor, RustEditor } from './util';
+import { PersistentState } from './persistent_state';
 
 export class Ctx {
     private constructor(
         readonly config: Config,
+        readonly state: PersistentState,
         private readonly extCtx: vscode.ExtensionContext,
         readonly client: lc.LanguageClient
     ) {
 
     }
 
-    static async create(config: Config, extCtx: vscode.ExtensionContext, serverPath: string): Promise<Ctx> {
+    static async create(config: Config, state: PersistentState, extCtx: vscode.ExtensionContext, serverPath: string): Promise<Ctx> {
         const client = await createClient(config, serverPath);
-        const res = new Ctx(config, extCtx, client);
+        const res = new Ctx(config, state, extCtx, client);
         res.pushCleanup(client.start());
         await client.onReady();
         return res;

editors/code/src/installation/extension.ts

@@ -7,6 +7,7 @@ import { Config, UpdatesChannel } from "../config";
 import { ArtifactReleaseInfo, ArtifactSource } from "./interfaces";
 import { downloadArtifactWithProgressUi } from "./downloads";
 import { fetchArtifactReleaseInfo } from "./fetch_artifact_release_info";
+import { PersistentState } from "../persistent_state";
 
 const HEURISTIC_NIGHTLY_RELEASE_PERIOD_IN_HOURS = 25;
 
@@ -14,7 +15,7 @@ const HEURISTIC_NIGHTLY_RELEASE_PERIOD_IN_HOURS = 25;
  * Installs `stable` or latest `nightly` version or does nothing if the current
  * extension version is what's needed according to `desiredUpdateChannel`.
  */
-export async function ensureProperExtensionVersion(config: Config): Promise<never | void> {
+export async function ensureProperExtensionVersion(config: Config, state: PersistentState): Promise<never | void> {
     // User has built lsp server from sources, she should manage updates manually
     if (config.serverSource?.type === ArtifactSource.Type.ExplicitPath) return;
 
@@ -23,7 +24,7 @@ export async function ensureProperExtensionVersion(config: Config): Promise<neve
 
     if (currentUpdChannel === UpdatesChannel.Stable) {
         // Release date is present only when we are on nightly
-        await config.installedNightlyExtensionReleaseDate.set(null);
+        await state.installedNightlyExtensionReleaseDate.set(null);
     }
 
     if (desiredUpdChannel === UpdatesChannel.Stable) {
@@ -39,10 +40,10 @@ export async function ensureProperExtensionVersion(config: Config): Promise<neve
     if (currentUpdChannel === UpdatesChannel.Stable) {
         if (!await askToDownloadProperExtensionVersion(config)) return;
 
-        return await tryDownloadNightlyExtension(config);
+        return await tryDownloadNightlyExtension(config, state);
     }
 
-    const currentExtReleaseDate = config.installedNightlyExtensionReleaseDate.get();
+    const currentExtReleaseDate = state.installedNightlyExtensionReleaseDate.get();
 
     if (currentExtReleaseDate === null) {
         void vscode.window.showErrorMessage(
@@ -66,9 +67,9 @@ export async function ensureProperExtensionVersion(config: Config): Promise<neve
         return;
     }
 
-    await tryDownloadNightlyExtension(config, releaseInfo => {
+    await tryDownloadNightlyExtension(config, state, releaseInfo => {
         assert(
-            currentExtReleaseDate.getTime() === config.installedNightlyExtensionReleaseDate.get()?.getTime(),
+            currentExtReleaseDate.getTime() === state.installedNightlyExtensionReleaseDate.get()?.getTime(),
             "Other active VSCode instance has reinstalled the extension"
         );
 
@@ -111,6 +112,7 @@ async function askToDownloadProperExtensionVersion(config: Config, reason = "")
  */
 const tryDownloadNightlyExtension = notReentrant(async (
     config: Config,
+    state: PersistentState,
     shouldDownload: (releaseInfo: ArtifactReleaseInfo) => boolean = () => true
 ): Promise<never | void> => {
     const vsixSource = config.nightlyVsixSource;
@@ -124,7 +126,7 @@ const tryDownloadNightlyExtension = notReentrant(async (
         const vsixPath = path.join(vsixSource.dir, vsixSource.file);
 
         await vscodeInstallExtensionFromVsix(vsixPath);
-        await config.installedNightlyExtensionReleaseDate.set(releaseInfo.releaseDate);
+        await state.installedNightlyExtensionReleaseDate.set(releaseInfo.releaseDate);
         await fs.unlink(vsixPath);
 
         await vscodeReloadWindow(); // never returns

editors/code/src/installation/server.ts

@@ -7,8 +7,9 @@ import { fetchArtifactReleaseInfo } from "./fetch_artifact_release_info";
 import { downloadArtifactWithProgressUi } from "./downloads";
 import { log, assert, notReentrant } from "../util";
 import { Config, NIGHTLY_TAG } from "../config";
+import { PersistentState } from "../persistent_state";
 
-export async function ensureServerBinary(config: Config): Promise<null | string> {
+export async function ensureServerBinary(config: Config, state: PersistentState): Promise<null | string> {
     const source = config.serverSource;
 
     if (!source) {
@@ -37,7 +38,7 @@ export async function ensureServerBinary(config: Config): Promise<null | string>
             return null;
         }
         case ArtifactSource.Type.GithubRelease: {
-            if (!shouldDownloadServer(source, config)) {
+            if (!shouldDownloadServer(state, source)) {
                 return path.join(source.dir, source.file);
             }
 
@@ -50,24 +51,24 @@ export async function ensureServerBinary(config: Config): Promise<null | string>
                 if (userResponse !== "Download now") return null;
             }
 
-            return await downloadServer(source, config);
+            return await downloadServer(state, source);
         }
     }
 }
 
 function shouldDownloadServer(
+    state: PersistentState,
     source: ArtifactSource.GithubRelease,
-    config: Config
 ): boolean {
     if (!isBinaryAvailable(path.join(source.dir, source.file))) return true;
 
     const installed = {
-        tag: config.serverReleaseTag.get(),
-        date: config.serverReleaseDate.get()
+        tag: state.serverReleaseTag.get(),
+        date: state.serverReleaseDate.get()
     };
     const required = {
         tag: source.tag,
-        date: config.installedNightlyExtensionReleaseDate.get()
+        date: state.installedNightlyExtensionReleaseDate.get()
     };
 
     log.debug("Installed server:", installed, "required:", required);
@@ -86,16 +87,16 @@ function shouldDownloadServer(
  * Enforcing no reentrancy for this is best-effort.
  */
 const downloadServer = notReentrant(async (
+    state: PersistentState,
     source: ArtifactSource.GithubRelease,
-    config: Config,
 ): Promise<null | string> => {
     try {
         const releaseInfo = await fetchArtifactReleaseInfo(source.repo, source.file, source.tag);
 
         await downloadArtifactWithProgressUi(releaseInfo, source.file, source.dir, "language server");
         await Promise.all([
-            config.serverReleaseTag.set(releaseInfo.releaseName),
-            config.serverReleaseDate.set(releaseInfo.releaseDate)
+            state.serverReleaseTag.set(releaseInfo.releaseName),
+            state.serverReleaseDate.set(releaseInfo.releaseDate)
         ]);
     } catch (err) {
         log.downloadError(err, "language server", source.repo.name);

editors/code/src/main.ts

@@ -9,6 +9,7 @@ import { ensureServerBinary } from './installation/server';
 import { Config } from './config';
 import { log } from './util';
 import { ensureProperExtensionVersion } from './installation/extension';
+import { PersistentState } from './persistent_state';
 
 let ctx: Ctx | undefined;
 
@@ -34,13 +35,14 @@ export async function activate(context: vscode.ExtensionContext) {
     context.subscriptions.push(defaultOnEnter);
 
     const config = new Config(context);
+    const state = new PersistentState(context);
 
-    vscode.workspace.onDidChangeConfiguration(() => ensureProperExtensionVersion(config).catch(log.error));
+    vscode.workspace.onDidChangeConfiguration(() => ensureProperExtensionVersion(config, state).catch(log.error));
 
     // Don't await the user response here, otherwise we will block the lsp server bootstrap
-    void ensureProperExtensionVersion(config).catch(log.error);
+    void ensureProperExtensionVersion(config, state).catch(log.error);
 
-    const serverPath = await ensureServerBinary(config);
+    const serverPath = await ensureServerBinary(config, state);
 
     if (serverPath == null) {
         throw new Error(
@@ -53,7 +55,7 @@ export async function activate(context: vscode.ExtensionContext) {
     // registers its `onDidChangeDocument` handler before us.
     //
     // This a horribly, horribly wrong way to deal with this problem.
-    ctx = await Ctx.create(config, context, serverPath);
+    ctx = await Ctx.create(config, state, context, serverPath);
 
     // Commands which invokes manually via command palette, shortcut, etc.
     ctx.registerCommand('reload', (ctx) => {

editors/code/src/persistent_state.ts

@@ -0,0 +1,49 @@
+import * as vscode from 'vscode';
+import { log } from "./util";
+
+export class PersistentState {
+    constructor(private readonly ctx: vscode.ExtensionContext) {
+    }
+
+    readonly installedNightlyExtensionReleaseDate = new DateStorage(
+        "installed-nightly-extension-release-date",
+        this.ctx.globalState
+    );
+    readonly serverReleaseDate = new DateStorage("server-release-date", this.ctx.globalState);
+    readonly serverReleaseTag = new Storage<null | string>("server-release-tag", this.ctx.globalState, null);
+}
+
+
+export class Storage<T> {
+    constructor(
+        private readonly key: string,
+        private readonly storage: vscode.Memento,
+        private readonly defaultVal: T
+    ) { }
+
+    get(): T {
+        const val = this.storage.get(this.key, this.defaultVal);
+        log.debug(this.key, "==", val);
+        return val;
+    }
+    async set(val: T) {
+        log.debug(this.key, "=", val);
+        await this.storage.update(this.key, val);
+    }
+}
+export class DateStorage {
+    inner: Storage<null | string>;
+
+    constructor(key: string, storage: vscode.Memento) {
+        this.inner = new Storage(key, storage, null);
+    }
+
+    get(): null | Date {
+        const dateStr = this.inner.get();
+        return dateStr ? new Date(dateStr) : null;
+    }
+
+    async set(date: null | Date) {
+        await this.inner.set(date ? date.toString() : null);
+    }
+}