Chore: Use promises for file system operations related to embedded la…

…nguage documents - This fixes tests that would give inconsistent results for file deletion - This handles the case where storagePath is undefined
savoirfairelinux · Nov 2, 2023 · e61f183 · e61f183
1 parent 3e918c6
commit e61f183
Show file tree

Hide file tree

Showing 7 changed files with 88 additions and 55 deletions.
diff --git a/integration-tests/project-folder/.vscode/settings.json b/integration-tests/project-folder/.vscode/settings.json
@@ -1,3 +1,6 @@
 {
     "bitbake.loggingLevel": "debug",
+    "editor.quickSuggestions": {
+        "strings": false
+    },
 }
diff --git a/server/src/__tests__/embedded-languages.test.ts b/server/src/__tests__/embedded-languages.test.ts
@@ -18,7 +18,7 @@ describe('Embedded Language Documents', () => {
       analyzer.initialize(parser)
     }
     analyzer.resetAnalyzedDocuments()
-    embeddedLanguageDocsManager.storagePath = __dirname
+    await embeddedLanguageDocsManager.setStoragePath(__dirname)
   })
 
   beforeEach(() => {
@@ -32,7 +32,7 @@ describe('Embedded Language Documents', () => {
       document: FIXTURE_DOCUMENT.EMBEDDED
     })
 
-    generateEmbeddedLanguageDocs(FIXTURE_DOCUMENT.EMBEDDED)
+    await generateEmbeddedLanguageDocs(FIXTURE_DOCUMENT.EMBEDDED)
 
     // Test embedded documents infos
     const bashEmbeddedLanguageDocInfos = embeddedLanguageDocsManager.getEmbeddedLanguageDocInfos(FIXTURE_DOCUMENT.EMBEDDED.uri, 'bash')
@@ -80,7 +80,7 @@ describe('Embedded Language Documents', () => {
     expect(pythonEmbeddedLanguageDocInfosOnPosition).toEqual(pythonEmbeddedLanguageDocInfos)
 
     // Test saving the document a second time does not create a new file
-    generateEmbeddedLanguageDocs(FIXTURE_DOCUMENT.EMBEDDED)
+    await generateEmbeddedLanguageDocs(FIXTURE_DOCUMENT.EMBEDDED)
     const bashEmbeddedLanguageDocInfos2 = embeddedLanguageDocsManager.getEmbeddedLanguageDocInfos(FIXTURE_DOCUMENT.EMBEDDED.uri, 'bash')
     expect(bashEmbeddedLanguageDocInfos2?.uri).toEqual(bashEmbeddedLanguageDocInfos.uri)
 
@@ -96,9 +96,9 @@ describe('Embedded Language Documents', () => {
     expect(embeddedLanguageDocsManager.getEmbeddedLanguageDocInfos(newUri, 'python')).toEqual(pythonEmbeddedLanguageDocInfos)
 
     // Test deletion
-    embeddedLanguageDocsManager.deleteEmbeddedLanguageDocs(newUri)
-    expect(() => fs.readFileSync(bashEmbeddedLanguageDocPath)).toThrow()
-    expect(() => fs.readFileSync(pythonEmbeddedLanguageDocPath)).toThrow()
+    await embeddedLanguageDocsManager.deleteEmbeddedLanguageDocs(newUri)
+    expect(fs.existsSync(bashEmbeddedLanguageDocPath)).toBeFalsy()
+    expect(fs.existsSync(pythonEmbeddedLanguageDocPath)).toBeFalsy()
   })
 })
 

diff --git a/server/src/embedded-languages/bash-support.ts b/server/src/embedded-languages/bash-support.ts
@@ -10,7 +10,7 @@ import { analyzer } from '../tree-sitter/analyzer'
 import { type EmbeddedLanguageDocInfos } from './utils'
 import { embeddedLanguageDocsManager } from './documents-manager'
 
-export const generateBashEmbeddedLanguageDoc = (textDocument: TextDocument): void => {
+export const generateBashEmbeddedLanguageDoc = async (textDocument: TextDocument): Promise<void> => {
   const bashRegions = analyzer.getBashRegions(textDocument.uri)
   const documentAsText = textDocument.getText().split(/\r?\n/g)
   const embeddedLanguageDocAsText = replaceTextForSpaces(documentAsText)
@@ -26,5 +26,5 @@ export const generateBashEmbeddedLanguageDoc = (textDocument: TextDocument): voi
     language: 'bash',
     lineOffset: 0
   }
-  embeddedLanguageDocsManager.saveEmbeddedLanguageDoc(textDocument.uri, content, partialInfos)
+  await embeddedLanguageDocsManager.saveEmbeddedLanguageDoc(textDocument.uri, content, partialInfos)
 }
diff --git a/server/src/embedded-languages/documents-manager.ts b/server/src/embedded-languages/documents-manager.ts
@@ -22,32 +22,48 @@ type EmbeddedLanguageDocsRecord = Partial<Record<EmbeddedLanguageType, EmbeddedL
 
 export default class EmbeddedLanguageDocsManager {
   private readonly embeddedLanguageDocsInfos = new Map<string, EmbeddedLanguageDocsRecord>() // map of original uri to embedded documents infos
-  private _storagePath: string = ''
+  private _storagePath: string | undefined
 
-  get storagePath (): string {
+  get storagePath (): string | undefined {
     return this._storagePath
   }
 
-  set storagePath (storagePath: string) {
-    logger.debug(`Set embedded language documents storage path. New: ${storagePath}. Old: ${this._storagePath}`)
-    if (this._storagePath === storagePath) {
+  async setStoragePath (newStoragePath: string | undefined): Promise<void> {
+    logger.debug(`Set embedded language documents storage path. New: ${newStoragePath}. Old: ${this._storagePath}`)
+    if (this._storagePath === newStoragePath) {
       return
     }
-    const oldPathToEmbeddedLanguageDocsFolder = path.join(this._storagePath, EMBEDDED_DOCUMENTS_FOLDER)
-    const newPathToEmbeddedLanguageDocsFolder = path.join(storagePath, EMBEDDED_DOCUMENTS_FOLDER)
     // Writing the code to move the existing files into the new folder is pointless optimization (and efforts):
     // In practice, storagePath is not intended to change.
-    try {
-      fs.mkdirSync(newPathToEmbeddedLanguageDocsFolder, { recursive: true })
-    } catch (error) {
-      logger.error('Failed to create embedded language documents folder:', error)
-    }
-    try {
-      fs.rmdirSync(oldPathToEmbeddedLanguageDocsFolder, { recursive: true })
-    } catch (error) {
-      logger.error('Failed to remove embedded language documents folder:', error)
-    }
-    this._storagePath = storagePath
+    await Promise.all([
+      new Promise<void>((resolve) => {
+        if (newStoragePath === undefined) {
+          resolve()
+          return
+        }
+        const newPathToEmbeddedLanguageDocsFolder = path.join(newStoragePath, EMBEDDED_DOCUMENTS_FOLDER)
+        fs.mkdir(newPathToEmbeddedLanguageDocsFolder, { recursive: true }, (err) => {
+          if (err !== null) {
+            logger.error('Failed to create embedded language documents folder:', err)
+          }
+          resolve()
+        })
+      }),
+      new Promise<void>((resolve) => {
+        if (this._storagePath === undefined) {
+          resolve()
+          return
+        }
+        const oldPathToEmbeddedLanguageDocsFolder = path.join(this._storagePath, EMBEDDED_DOCUMENTS_FOLDER)
+        fs.rmdir(oldPathToEmbeddedLanguageDocsFolder, { recursive: true }, (err) => {
+          if (err !== null) {
+            logger.error('Failed to remove embedded language documents folder:', err)
+          }
+          resolve()
+        })
+      })
+    ])
+    this._storagePath = newStoragePath
   }
 
   private registerEmbeddedLanguageDocInfos (originalUriString: string, embeddedLanguageDocInfos: EmbeddedLanguageDocInfos): void {
@@ -64,7 +80,10 @@ export default class EmbeddedLanguageDocsManager {
     return embeddedLanguageDocs?.[languageType]
   }
 
-  private getPathToEmbeddedLanguageDoc (originalUriString: string, languageType: EmbeddedLanguageType): string {
+  private getPathToEmbeddedLanguageDoc (originalUriString: string, languageType: EmbeddedLanguageType): string | undefined {
+    if (this.storagePath === undefined) {
+      return undefined
+    }
     const embeddedLanguageDocInfos = this.getEmbeddedLanguageDocInfos(originalUriString, languageType)
     if (embeddedLanguageDocInfos !== undefined) {
       return embeddedLanguageDocInfos.uri.replace('file://', '')
@@ -76,40 +95,49 @@ export default class EmbeddedLanguageDocsManager {
     return `${pathToEmbeddedLanguageDocsFolder}/${embeddedLanguageDocFilename}`
   }
 
-  saveEmbeddedLanguageDoc (
+  async saveEmbeddedLanguageDoc (
     originalUriString: string,
     embeddedLanguageDocContent: string,
     partialEmbeddedLanguageDocInfos: Omit<EmbeddedLanguageDocInfos, 'uri'>
-  ): void {
+  ): Promise<void> {
     logger.debug(`Save embedded document (${partialEmbeddedLanguageDocInfos.language}) for`, originalUriString)
     const pathToEmbeddedLanguageDoc = this.getPathToEmbeddedLanguageDoc(
       originalUriString,
       partialEmbeddedLanguageDocInfos.language
     )
-    try {
-      fs.writeFileSync(pathToEmbeddedLanguageDoc, embeddedLanguageDocContent)
-    } catch (error) {
-      logger.error('Failed to create embedded document:', error)
-    }
-    const documentInfos = {
-      ...partialEmbeddedLanguageDocInfos,
-      uri: `file://${pathToEmbeddedLanguageDoc}`
+    if (pathToEmbeddedLanguageDoc === undefined) {
+      return
     }
-    this.registerEmbeddedLanguageDocInfos(originalUriString, documentInfos)
+    await new Promise<void>((resolve, reject) => {
+      fs.writeFile(pathToEmbeddedLanguageDoc, embeddedLanguageDocContent, (err) => {
+        err !== null ? reject(err) : resolve()
+      })
+    }).then(() => {
+      const documentInfos = {
+        ...partialEmbeddedLanguageDocInfos,
+        uri: `file://${pathToEmbeddedLanguageDoc}`
+      }
+      this.registerEmbeddedLanguageDocInfos(originalUriString, documentInfos)
+    }).catch((err) => {
+      logger.error('Failed to create embedded document:', err)
+    })
   }
 
-  deleteEmbeddedLanguageDocs (originalUriString: string): void {
+  async deleteEmbeddedLanguageDocs (originalUriString: string): Promise<void> {
     logger.debug('Delete embedded documents for', originalUriString)
     const embeddedLanguageDocs = this.embeddedLanguageDocsInfos.get(originalUriString) ?? {}
-    Object.values(embeddedLanguageDocs).forEach(({ uri }) => {
-      const pathToEmbeddedLanguageDoc = uri.replace('file://', '')
-      try {
-        fs.unlink(pathToEmbeddedLanguageDoc, () => {})
-      } catch (error) {
-        logger.error('Failed to delete embedded document:', error)
-      }
+    await Promise.all(Object.values(embeddedLanguageDocs).map(async ({ uri }) => {
+      await new Promise<void>((resolve, reject) => {
+        const pathToEmbeddedLanguageDoc = uri.replace('file://', '')
+        fs.unlink(pathToEmbeddedLanguageDoc, (err) => {
+          err !== null ? reject(err) : resolve()
+        })
+      })
+    })).then(() => {
+      this.embeddedLanguageDocsInfos.delete(originalUriString)
+    }).catch((err) => {
+      logger.error('Failed to delete embedded document:', err)
     })
-    this.embeddedLanguageDocsInfos.delete(originalUriString)
   }
 
   renameEmbeddedLanguageDocs (oldUriString: string, newUriString: string): void {

diff --git a/server/src/embedded-languages/general-support.ts b/server/src/embedded-languages/general-support.ts
@@ -11,9 +11,11 @@ import { generatePythonEmbeddedLanguageDoc } from './python-support'
 import { embeddedLanguageDocsManager } from './documents-manager'
 import { isInsideBashRegion, isInsidePythonRegion } from './utils'
 
-export const generateEmbeddedLanguageDocs = (textDocument: TextDocument): void => {
-  generateBashEmbeddedLanguageDoc(textDocument)
-  generatePythonEmbeddedLanguageDoc(textDocument)
+export const generateEmbeddedLanguageDocs = async (textDocument: TextDocument): Promise<void> => {
+  await Promise.all([
+    generateBashEmbeddedLanguageDoc(textDocument),
+    generatePythonEmbeddedLanguageDoc(textDocument)
+  ])
 }
 
 export const getEmbeddedLanguageDocInfosOnPosition = (uriString: string, position: Position): { uri: string, lineOffset: number } | undefined => {

diff --git a/server/src/embedded-languages/python-support.ts b/server/src/embedded-languages/python-support.ts
@@ -10,7 +10,7 @@ import { analyzer } from '../tree-sitter/analyzer'
 import { embeddedLanguageDocsManager } from './documents-manager'
 import { type EmbeddedLanguageDocInfos } from './utils'
 
-export const generatePythonEmbeddedLanguageDoc = (textDocument: TextDocument): void => {
+export const generatePythonEmbeddedLanguageDoc = async (textDocument: TextDocument): Promise<void> => {
   const pythonRegions = analyzer.getPythonRegions(textDocument.uri)
   const documentAsText = textDocument.getText().split(/\r?\n/g)
   const embeddedLanguageDocAsText = replaceTextForSpaces(documentAsText)
@@ -31,5 +31,5 @@ export const generatePythonEmbeddedLanguageDoc = (textDocument: TextDocument): v
     language: 'python',
     lineOffset: 1
   }
-  embeddedLanguageDocsManager.saveEmbeddedLanguageDoc(textDocument.uri, content, partialInfos)
+  await embeddedLanguageDocsManager.saveEmbeddedLanguageDoc(textDocument.uri, content, partialInfos)
 }
diff --git a/server/src/server.ts b/server/src/server.ts
@@ -43,7 +43,7 @@ connection.onInitialize(async (params: InitializeParams): Promise<InitializeResu
   setNotificationManagerConnection(connection)
 
   const storagePath = params.initializationOptions.storagePath as string
-  embeddedLanguageDocsManager.storagePath = storagePath
+  await embeddedLanguageDocsManager.setStoragePath(storagePath)
 
   const parser = await generateParser()
   analyzer.initialize(parser)
@@ -95,7 +95,7 @@ connection.onDidChangeWatchedFiles((change) => {
   logger.debug(`onDidChangeWatchedFiles: ${JSON.stringify(change)}`)
   change.changes?.forEach((change) => {
     if (change.type === FileChangeType.Deleted) {
-      embeddedLanguageDocsManager.deleteEmbeddedLanguageDocs(change.uri)
+      void embeddedLanguageDocsManager.deleteEmbeddedLanguageDocs(change.uri)
     }
   })
   bitBakeProjectScanner.rescanProject()
@@ -138,7 +138,7 @@ documents.onDidChangeContent(async (event) => {
 
   if (textDocument.getText().length > 0) {
     const diagnostics = await analyzer.analyze({ document: textDocument, uri: textDocument.uri })
-    generateEmbeddedLanguageDocs(event.document)
+    void generateEmbeddedLanguageDocs(event.document)
     void connection.sendDiagnostics({ uri: textDocument.uri, diagnostics })
   }