Merge pull request #569 from bash-lsp/performant-background-analysis

skovhus · web-flow · commit a8e33fb73ec9 · 2022-11-28T09:46:53.000+01:00
Performant globbing and background analysis
diff --git a/server/package.json b/server/package.json
@@ -19,8 +19,8 @@
   },
   "dependencies": {
     "@types/node-fetch": "2.6.2",
+    "fast-glob": "3.2.12",
     "fuzzy-search": "^3.2.1",
-    "glob": "^8.0.0",
     "node-fetch": "^2.6.7",
     "turndown": "^7.0.0",
     "urijs": "^1.19.11",
diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts
@@ -7,10 +7,14 @@ import * as fsUtil from '../util/fs'
 let analyzer: Analyzer
 
 const CURRENT_URI = 'dummy-uri.sh'
+const mockConsole = getMockConnection().console
+
+// if you add a .sh file to testing/fixtures, update this value
+const FIXTURE_FILES_MATCHING_GLOB = 13
 
 beforeAll(async () => {
   const parser = await initializeParser()
-  analyzer = new Analyzer(parser)
+  analyzer = new Analyzer({ console: mockConsole, parser })
 })
 
 describe('analyze', () => {
@@ -262,39 +266,28 @@ describe('commentsAbove', () => {
   })
 })
 
-describe('fromRoot', () => {
-  it('initializes an analyzer from a root', async () => {
+describe('initiateBackgroundAnalysis', () => {
+  it('finds bash files', async () => {
     const parser = await initializeParser()
 
     jest.spyOn(Date, 'now').mockImplementation(() => 0)
 
     const connection = getMockConnection()
 
-    const newAnalyzer = await Analyzer.fromRoot({
-      connection,
+    const newAnalyzer = new Analyzer({ console: connection.console, parser })
+    const { filesParsed } = await newAnalyzer.initiateBackgroundAnalysis({
       rootPath: FIXTURE_FOLDER,
-      parser,
     })
 
-    expect(newAnalyzer).toBeDefined()
-
     expect(connection.window.showWarningMessage).not.toHaveBeenCalled()
-
-    // if you add a .sh file to testing/fixtures, update this value
-    const FIXTURE_FILES_MATCHING_GLOB = 14
+    expect(connection.console.warn).not.toHaveBeenCalled()
 
     // Intro, stats on glob, one file skipped due to shebang, and outro
-    const LOG_LINES = FIXTURE_FILES_MATCHING_GLOB + 4
+    expect(filesParsed).toEqual(FIXTURE_FILES_MATCHING_GLOB)
 
-    expect(connection.console.log).toHaveBeenCalledTimes(LOG_LINES)
     expect(connection.console.log).toHaveBeenNthCalledWith(
       1,
-      expect.stringContaining('Analyzing files matching'),
-    )
-
-    expect(connection.console.log).toHaveBeenNthCalledWith(
-      LOG_LINES,
-      'Analyzer finished after 0 seconds',
+      expect.stringContaining('BackgroundAnalysis: resolving glob'),
     )
   })
 
@@ -307,16 +300,13 @@ describe('fromRoot', () => {
 
     const connection = getMockConnection()
 
-    const newAnalyzer = await Analyzer.fromRoot({
-      connection,
+    const newAnalyzer = new Analyzer({ console: connection.console, parser })
+    const { filesParsed } = await newAnalyzer.initiateBackgroundAnalysis({
       rootPath: FIXTURE_FOLDER,
-      parser,
     })
 
-    expect(newAnalyzer).toBeDefined()
-
-    expect(connection.window.showWarningMessage).toHaveBeenCalledWith(
-      expect.stringContaining('BOOM'),
-    )
+    expect(connection.window.showWarningMessage).not.toHaveBeenCalled()
+    expect(connection.console.warn).toHaveBeenCalledWith(expect.stringContaining('BOOM'))
+    expect(filesParsed).toEqual(0)
   })
 })
diff --git a/server/src/__tests__/server.test.ts b/server/src/__tests__/server.test.ts
@@ -20,6 +20,12 @@ async function initializeServer() {
     workspaceFolders: null,
   })
 
+  const { backgroundAnalysisCompleted } = server
+
+  if (backgroundAnalysisCompleted) {
+    await backgroundAnalysisCompleted
+  }
+
   return {
     connection,
     console,
diff --git a/server/src/analyser.ts b/server/src/analyser.ts
@@ -7,7 +7,7 @@ import { promisify } from 'util'
 import * as LSP from 'vscode-languageserver'
 import * as Parser from 'web-tree-sitter'
 
-import { getGlobPattern } from './config'
+import * as config from './config'
 import { flattenArray, flattenObjectValues } from './util/flatten'
 import { getFilePaths } from './util/fs'
 import { getShebang, isBashShebang } from './util/shebang'
@@ -28,77 +28,8 @@ type Texts = { [uri: string]: string }
  * tree-sitter to find definitions, reference, etc.
  */
 export default class Analyzer {
-  /**
-   * Initialize the Analyzer based on a connection to the client and an optional
-   * root path.
-   *
-   * If the rootPath is provided it will initialize all shell files it can find
-   * anywhere on that path. This non-exhaustive glob is used to preload the parser
-   * to support features across files.
-   */
-  public static async fromRoot({
-    connection,
-    rootPath,
-    parser,
-  }: {
-    connection: LSP.Connection
-    rootPath: LSP.InitializeParams['rootPath']
-    parser: Parser
-  }): Promise<Analyzer> {
-    const analyzer = new Analyzer(parser)
-
-    if (rootPath) {
-      const globPattern = getGlobPattern()
-      connection.console.log(
-        `Analyzing files matching glob "${globPattern}" inside ${rootPath}`,
-      )
-
-      const lookupStartTime = Date.now()
-      const getTimePassed = (): string =>
-        `${(Date.now() - lookupStartTime) / 1000} seconds`
-
-      let filePaths: string[] = []
-      try {
-        filePaths = await getFilePaths({ globPattern, rootPath })
-      } catch (error) {
-        const errorMessage = error instanceof Error ? error.message : error
-        connection.window.showWarningMessage(
-          `Failed to analyze bash files using the glob "${globPattern}". The experience will be degraded. Error: ${errorMessage}`,
-        )
-      }
-
-      // TODO: we could load all files without extensions: globPattern: '**/[^.]'
-
-      connection.console.log(
-        `Glob resolved with ${filePaths.length} files after ${getTimePassed()}`,
-      )
-
-      for (const filePath of filePaths) {
-        const uri = url.pathToFileURL(filePath).href
-        connection.console.log(`Analyzing ${uri}`)
-
-        try {
-          const fileContent = await readFileAsync(filePath, 'utf8')
-          const shebang = getShebang(fileContent)
-          if (shebang && !isBashShebang(shebang)) {
-            connection.console.log(`Skipping file ${uri} with shebang "${shebang}"`)
-            continue
-          }
-
-          analyzer.analyze(uri, LSP.TextDocument.create(uri, 'shell', 1, fileContent))
-        } catch (error) {
-          const errorMessage = error instanceof Error ? error.message : error
-          connection.console.warn(`Failed analyzing ${uri}. Error: ${errorMessage}`)
-        }
-      }
-
-      connection.console.log(`Analyzer finished after ${getTimePassed()}`)
-    }
-
-    return analyzer
-  }
-
   private parser: Parser
+  private console: LSP.RemoteConsole
 
   private uriToTextDocument: { [uri: string]: LSP.TextDocument } = {}
 
@@ -116,8 +47,82 @@ export default class Analyzer {
     variable_assignment: LSP.SymbolKind.Variable,
   }
 
-  public constructor(parser: Parser) {
+  public constructor({
+    console,
+    parser,
+  }: {
+    console: LSP.RemoteConsole
+    parser: Parser
+  }) {
     this.parser = parser
+    this.console = console
+  }
+
+  /**
+   * Initiates a background analysis of the files in the given rootPath to
+   * enable features across files.
+   */
+  public async initiateBackgroundAnalysis({
+    rootPath,
+  }: {
+    rootPath: string
+  }): Promise<{ filesParsed: number }> {
+    const globPattern = config.getGlobPattern()
+    const backgroundAnalysisMaxFiles = config.getBackgroundAnalysisMaxFiles()
+    this.console.log(
+      `BackgroundAnalysis: resolving glob "${globPattern}" inside "${rootPath}"...`,
+    )
+
+    const lookupStartTime = Date.now()
+    const getTimePassed = (): string => `${(Date.now() - lookupStartTime) / 1000} seconds`
+
+    let filePaths: string[] = []
+    try {
+      filePaths = await getFilePaths({
+        globPattern,
+        rootPath,
+        maxItems: backgroundAnalysisMaxFiles,
+      })
+    } catch (error) {
+      const errorMessage = error instanceof Error ? error.message : error
+      this.console.warn(
+        `BackgroundAnalysis: failed resolved glob "${globPattern}". The experience across files will be degraded. Error: ${errorMessage}`,
+      )
+      return { filesParsed: 0 }
+    }
+
+    this.console.log(
+      `BackgroundAnalysis: Glob resolved with ${
+        filePaths.length
+      } files after ${getTimePassed()}`,
+    )
+
+    for (const filePath of filePaths) {
+      const uri = url.pathToFileURL(filePath).href
+
+      try {
+        const fileContent = await readFileAsync(filePath, 'utf8')
+        const shebang = getShebang(fileContent)
+        if (shebang && !isBashShebang(shebang)) {
+          this.console.log(
+            `BackgroundAnalysis: Skipping file ${uri} with shebang "${shebang}"`,
+          )
+          continue
+        }
+
+        this.analyze(uri, LSP.TextDocument.create(uri, 'shell', 1, fileContent))
+      } catch (error) {
+        const errorMessage = error instanceof Error ? error.message : error
+        this.console.warn(
+          `BackgroundAnalysis: Failed analyzing ${uri}. Error: ${errorMessage}`,
+        )
+      }
+    }
+
+    this.console.log(`BackgroundAnalysis: Completed after ${getTimePassed()}.`)
+    return {
+      filesParsed: filePaths.length,
+    }
   }
 
   /**
diff --git a/server/src/config.ts b/server/src/config.ts
@@ -1,4 +1,5 @@
 export const DEFAULT_GLOB_PATTERN = '**/*@(.sh|.inc|.bash|.command)'
+export const DEFAULT_BACKGROUND_ANALYSIS_MAX_FILES = 500
 
 export function getShellcheckPath(): string | null {
   const { SHELLCHECK_PATH } = process.env
@@ -13,6 +14,9 @@ export function getExplainshellEndpoint(): string | null {
     : null
 }
 
+/**
+ * Get the glob pattern for files to run background analysis on.
+ */
 export function getGlobPattern(): string {
   const { GLOB_PATTERN } = process.env
   return typeof GLOB_PATTERN === 'string' && GLOB_PATTERN.trim() !== ''
@@ -26,3 +30,12 @@ export function getHighlightParsingError(): boolean {
     ? HIGHLIGHT_PARSING_ERRORS === 'true' || HIGHLIGHT_PARSING_ERRORS === '1'
     : false
 }
+
+/**
+ * Get the maximum number of files to run background analysis on.
+ */
+export function getBackgroundAnalysisMaxFiles(): number {
+  const { BACKGROUND_ANALYSIS_MAX_FILES } = process.env
+  const parsed = parseInt(BACKGROUND_ANALYSIS_MAX_FILES || '', 10)
+  return !isNaN(parsed) ? parsed : DEFAULT_BACKGROUND_ANALYSIS_MAX_FILES
+}
diff --git a/server/src/server.ts b/server/src/server.ts
@@ -38,14 +38,27 @@ export default class BashServer {
       throw new Error('Expected PATH environment variable to be set')
     }
 
-    return Promise.all([
-      Executables.fromPath(PATH),
-      Analyzer.fromRoot({ connection, rootPath, parser }),
-      getLinterExecutablePath(),
-    ]).then(([executables, analyzer, linterExecutablePath]) => {
-      const linter = new Linter({ executablePath: linterExecutablePath })
-      return new BashServer(connection, executables, analyzer, linter, capabilities)
-    })
+    return Promise.all([Executables.fromPath(PATH), getLinterExecutablePath()]).then(
+      ([executables, linterExecutablePath]) => {
+        const analyzer = new Analyzer({ console: connection.console, parser })
+        const linter = new Linter({ executablePath: linterExecutablePath })
+
+        let backgroundAnalysisCompleted
+        if (rootPath) {
+          // NOTE: we do not block the server initialization on this background analysis.
+          backgroundAnalysisCompleted = analyzer.initiateBackgroundAnalysis({ rootPath })
+        }
+
+        return new BashServer({
+          analyzer,
+          backgroundAnalysisCompleted,
+          capabilities,
+          connection,
+          executables,
+          linter,
+        })
+      },
+    )
   }
 
   private executables: Executables
@@ -55,19 +68,29 @@ export default class BashServer {
   private documents: LSP.TextDocuments<TextDocument> = new LSP.TextDocuments(TextDocument)
   private connection: LSP.Connection
   private clientCapabilities: LSP.ClientCapabilities
-
-  private constructor(
-    connection: LSP.Connection,
-    executables: Executables,
-    analyzer: Analyzer,
-    linter: Linter,
-    capabilities: LSP.ClientCapabilities,
-  ) {
+  public backgroundAnalysisCompleted?: Promise<any>
+
+  private constructor({
+    connection,
+    executables,
+    analyzer,
+    linter,
+    capabilities,
+    backgroundAnalysisCompleted,
+  }: {
+    connection: LSP.Connection
+    executables: Executables
+    analyzer: Analyzer
+    linter: Linter
+    capabilities: LSP.ClientCapabilities
+    backgroundAnalysisCompleted?: Promise<any>
+  }) {
     this.connection = connection
     this.executables = executables
     this.analyzer = analyzer
     this.linter = linter
     this.clientCapabilities = capabilities
+    this.backgroundAnalysisCompleted = backgroundAnalysisCompleted
   }
 
   /**
diff --git a/server/src/util/fs.ts b/server/src/util/fs.ts
diff --git a/server/yarn.lock b/server/yarn.lock
diff --git a/vscode-client/yarn.lock b/vscode-client/yarn.lock
