Merge pull request mathworks#38 from development/dlilley.refactor.reverse-dependencies

Refactor to improve dependencies and testability of code

Author: dklilley

src/ClientConnection.ts

@@ -0,0 +1,32 @@
+// Copyright 2024 The MathWorks, Inc.
+import { _Connection, createConnection, ProposedFeatures } from "vscode-languageserver/node"
+
+export type Connection = _Connection
+
+export default class ClientConnection {
+    private static connection: Connection
+
+    /**
+     * Retrieves the connection to the client. If no connection currently exists,
+     * a new connection is created.
+     *
+     * @returns The connection to the client
+     */
+    public static getConnection (): Connection {
+        if (ClientConnection.connection == null) {
+            ClientConnection.connection = createConnection(ProposedFeatures.all)
+        }
+
+        return ClientConnection.connection
+    }
+
+    /**
+     * Sets the ClientConnection to a given object.
+     * This API is primarily meant for testing purposes.
+     *
+     * @param connection The connection object to set
+     */
+    public static setConnection (connection: Connection): void {
+        ClientConnection.connection = connection
+    }
+}

src/index.ts

@@ -1,7 +1,11 @@
-// Copyright 2022 - 2023 The MathWorks, Inc.
+// Copyright 2022 - 2024 The MathWorks, Inc.
 
 // Start up the LSP server
-import { connection } from './server'
+import ClientConnection from './ClientConnection'
+import * as server from './server'
 
-// Listen on the connection
-connection.listen()
+// Start up the language server
+server.startServer()
+
+// Listen on the client connection
+ClientConnection.getConnection().listen()

src/indexing/DocumentIndexer.ts

@@ -10,9 +10,11 @@ const INDEXING_DELAY = 500 // Delay (in ms) after keystroke before attempting to
  * Handles indexing a currently open document to gather data about classes,
  * functions, and variables.
  */
-class DocumentIndexer {
+export default class DocumentIndexer {
     private readonly pendingFilesToIndex = new Map<string, NodeJS.Timeout>()
 
+    constructor (private indexer: Indexer) {}
+
     /**
      * Queues a document to be indexed. This handles debouncing so that
      * indexing is not performed on every keystroke.
@@ -36,7 +38,7 @@ class DocumentIndexer {
      * @param textDocument The document being indexed
      */
     indexDocument (textDocument: TextDocument): void {
-        void Indexer.indexDocument(textDocument)
+        void this.indexer.indexDocument(textDocument)
     }
 
     /**
@@ -63,12 +65,10 @@ class DocumentIndexer {
         const uri = textDocument.uri
         if (this.pendingFilesToIndex.has(uri)) {
             this.clearTimerForDocumentUri(uri)
-            await Indexer.indexDocument(textDocument)
+            await this.indexer.indexDocument(textDocument)
         }
         if (!FileInfoIndex.codeDataCache.has(uri)) {
-            await Indexer.indexDocument(textDocument)
+            await this.indexer.indexDocument(textDocument)
         }
     }
 }
-
-export default new DocumentIndexer()

src/indexing/FileInfoIndex.ts

@@ -95,6 +95,8 @@ export enum FunctionVisibility {
  * Serves as an cache of data extracted from files
  */
 class FileInfoIndex {
+    private static instance: FileInfoIndex
+
     /**
      * Maps document URI to the code data
      */
@@ -105,6 +107,14 @@ class FileInfoIndex {
      */
     readonly classInfoCache = new Map<string, MatlabClassInfo>()
 
+    public static getInstance (): FileInfoIndex {
+        if (FileInfoIndex.instance == null) {
+            FileInfoIndex.instance = new FileInfoIndex()
+        }
+
+        return FileInfoIndex.instance
+    }
+
     /**
      * Parses the raw data into a more usable form. Caches the resulting data
      * in the code data index.
@@ -489,4 +499,4 @@ function convertRange (codeDataRange: CodeDataRange): Range {
     )
 }
 
-export default new FileInfoIndex()
+export default FileInfoIndex.getInstance()

src/indexing/Indexer.ts

@@ -14,20 +14,22 @@ interface WorkspaceFileIndexedResponse {
     codeData: RawCodeData
 }
 
-class Indexer {
+export default class Indexer {
     private readonly INDEX_DOCUMENT_REQUEST_CHANNEL = '/matlabls/indexDocument/request'
     private readonly INDEX_DOCUMENT_RESPONSE_CHANNEL = '/matlabls/indexDocument/response'
 
     private readonly INDEX_FOLDERS_REQUEST_CHANNEL = '/matlabls/indexFolders/request'
     private readonly INDEX_FOLDERS_RESPONSE_CHANNEL = '/matlabls/indexFolders/response'
 
+    constructor (private matlabLifecycleManager: MatlabLifecycleManager, private pathResolver: PathResolver) {}
+
     /**
      * Indexes the given TextDocument and caches the data.
      *
      * @param textDocument The document being indexed
      */
     async indexDocument (textDocument: TextDocument): Promise<void> {
-        const matlabConnection = await MatlabLifecycleManager.getMatlabConnection()
+        const matlabConnection = await this.matlabLifecycleManager.getMatlabConnection()
 
         if (matlabConnection == null) {
             return
@@ -46,7 +48,7 @@ class Indexer {
      * @param folders A list of folder URIs to be indexed
      */
     async indexFolders (folders: string[]): Promise<void> {
-        const matlabConnection = await MatlabLifecycleManager.getMatlabConnection()
+        const matlabConnection = await this.matlabLifecycleManager.getMatlabConnection()
 
         if (matlabConnection == null) {
             return
@@ -79,7 +81,7 @@ class Indexer {
      * @param uri The URI for the file being indexed
      */
     async indexFile (uri: string): Promise<void> {
-        const matlabConnection = await MatlabLifecycleManager.getMatlabConnection()
+        const matlabConnection = await this.matlabLifecycleManager.getMatlabConnection()
 
         if (matlabConnection == null) {
             return
@@ -143,7 +145,7 @@ class Indexer {
         // Find and queue indexing for parent classes
         const baseClasses = parsedCodeData.classInfo.baseClasses
 
-        const resolvedBaseClasses = await PathResolver.resolvePaths(baseClasses, uri, matlabConnection)
+        const resolvedBaseClasses = await this.pathResolver.resolvePaths(baseClasses, uri, matlabConnection)
 
         resolvedBaseClasses.forEach(resolvedBaseClass => {
             const uri = resolvedBaseClass.uri
@@ -153,5 +155,3 @@ class Indexer {
         })
     }
 }
-
-export default new Indexer()

src/indexing/WorkspaceIndexer.ts

@@ -1,17 +1,19 @@
-// Copyright 2022 - 2023 The MathWorks, Inc.
+// Copyright 2022 - 2024 The MathWorks, Inc.
 
 import { ClientCapabilities, WorkspaceFolder, WorkspaceFoldersChangeEvent } from 'vscode-languageserver'
 import ConfigurationManager from '../lifecycle/ConfigurationManager'
-import { connection } from '../server'
 import Indexer from './Indexer'
+import ClientConnection from '../ClientConnection'
 
 /**
  * Handles indexing files in the user's workspace to gather data about classes,
  * functions, and variables.
  */
-class WorkspaceIndexer {
+export default class WorkspaceIndexer {
     private isWorkspaceIndexingSupported = false
 
+    constructor (private indexer: Indexer) {}
+
     /**
      * Sets up workspace change listeners, if supported.
      *
@@ -26,7 +28,7 @@ class WorkspaceIndexer {
             return
         }
 
-        connection.workspace.onDidChangeWorkspaceFolders((params: WorkspaceFoldersChangeEvent) => {
+        ClientConnection.getConnection().workspace.onDidChangeWorkspaceFolders((params: WorkspaceFoldersChangeEvent) => {
             void this.handleWorkspaceFoldersAdded(params.added)
         })
     }
@@ -39,13 +41,13 @@ class WorkspaceIndexer {
             return
         }
 
-        const folders = await connection.workspace.getWorkspaceFolders()
+        const folders = await ClientConnection.getConnection().workspace.getWorkspaceFolders()
 
         if (folders == null) {
             return
         }
 
-        void Indexer.indexFolders(folders.map(folder => folder.uri))
+        void this.indexer.indexFolders(folders.map(folder => folder.uri))
     }
 
     /**
@@ -58,7 +60,7 @@ class WorkspaceIndexer {
             return
         }
 
-        void Indexer.indexFolders(folders.map(folder => folder.uri))
+        void this.indexer.indexFolders(folders.map(folder => folder.uri))
     }
 
     /**
@@ -73,5 +75,3 @@ class WorkspaceIndexer {
         return this.isWorkspaceIndexingSupported && shouldIndexWorkspace
     }
 }
-
-export default new WorkspaceIndexer()

src/lifecycle/ConfigurationManager.ts

@@ -1,9 +1,9 @@
-// Copyright 2022 - 2023 The MathWorks, Inc.
+// Copyright 2022 - 2024 The MathWorks, Inc.
 
 import { ClientCapabilities, DidChangeConfigurationNotification, DidChangeConfigurationParams } from 'vscode-languageserver'
 import { reportTelemetrySettingsChange } from '../logging/TelemetryUtils'
-import { connection } from '../server'
 import { getCliArgs } from '../utils/CliUtils'
+import ClientConnection from '../ClientConnection'
 
 export enum Argument {
     // Basic arguments
@@ -45,6 +45,8 @@ const SETTING_NAMES: SettingName[] = [
 ]
 
 class ConfigurationManager {
+    private static instance: ConfigurationManager
+
     private configuration: Settings | null = null
     private readonly defaultConfiguration: Settings
     private globalSettings: Settings
@@ -77,12 +79,22 @@ class ConfigurationManager {
         }
     }
 
+    public static getInstance (): ConfigurationManager {
+        if (ConfigurationManager.instance == null) {
+            ConfigurationManager.instance = new ConfigurationManager
+        }
+
+        return ConfigurationManager.instance
+    }
+
     /**
      * Sets up the configuration manager
      *
      * @param capabilities The client capabilities
      */
     setup (capabilities: ClientCapabilities): void {
+        const connection = ClientConnection.getConnection()
+
         this.hasConfigurationCapability = capabilities.workspace?.configuration != null
 
         if (this.hasConfigurationCapability) {
@@ -101,6 +113,7 @@ class ConfigurationManager {
     async getConfiguration (): Promise<Settings> {
         if (this.hasConfigurationCapability) {
             if (this.configuration == null) {
+                const connection = ClientConnection.getConnection()
                 this.configuration = await connection.workspace.getConfiguration('MATLAB') as Settings
             }
 
@@ -164,4 +177,4 @@ class ConfigurationManager {
     }
 }
 
-export default new ConfigurationManager()
+export default ConfigurationManager.getInstance()

src/lifecycle/LifecycleNotificationHelper.ts

@@ -4,8 +4,18 @@ import NotificationService, { Notification } from '../notifications/Notification
 import { ConnectionState } from './MatlabSession'
 
 class LifecycleNotificationHelper {
+    private static instance: LifecycleNotificationHelper
+
     didMatlabLaunchFail = false
 
+    public static getInstance (): LifecycleNotificationHelper {
+        if (LifecycleNotificationHelper.instance == null) {
+            LifecycleNotificationHelper.instance = new LifecycleNotificationHelper()
+        }
+
+        return LifecycleNotificationHelper.instance
+    }
+
     /**
      * Sends notification to the language client of a change in the MATLAB® connection state.
      *
@@ -27,4 +37,4 @@ class LifecycleNotificationHelper {
     }
 }
 
-export default new LifecycleNotificationHelper()
+export default LifecycleNotificationHelper.getInstance()

src/lifecycle/MatlabLifecycleManager.ts

@@ -6,7 +6,7 @@ import ConfigurationManager, { Argument, ConnectionTiming } from "./Configuratio
 import { MatlabConnection } from "./MatlabCommunicationManager"
 import MatlabSession, { launchNewMatlab, connectToMatlab } from './MatlabSession'
 
-class MatlabLifecycleManager {
+export default class MatlabLifecycleManager {
     eventEmitter = new EventEmitter()
 
     private matlabSession: MatlabSession | null = null
@@ -182,5 +182,3 @@ function shouldConnectToRemoteMatlab (): boolean {
     // Assume we should connect to existing MATLAB if the matlabUrl startup flag has been provided
     return Boolean(ConfigurationManager.getArgument(Argument.MatlabUrl))
 }
-
-export default new MatlabLifecycleManager()