Merge pull request #686 from bash-lsp/remove-parsing-errors

Move infrequent and rather useless parsing error to logger

Author: skovhus

server/src/__tests__/analyzer.test.ts

@@ -42,60 +42,29 @@ describe('analyze', () => {
       document: FIXTURE_DOCUMENT.INSTALL,
     })
     expect(diagnostics).toEqual([])
+    expect(loggerWarn).not.toHaveBeenCalled()
   })
 
-  it('returns a list of diagnostics for a file with a missing node', () => {
+  it('parses files with a missing node', () => {
     const diagnostics = analyzer.analyze({
       uri: CURRENT_URI,
       document: FIXTURE_DOCUMENT.MISSING_NODE,
     })
-    expect(diagnostics).not.toEqual([])
-    expect(diagnostics).toMatchInlineSnapshot(`
-      Array [
-        Object {
-          "message": "Syntax error: expected \\"fi\\" somewhere in the file",
-          "range": Object {
-            "end": Object {
-              "character": 0,
-              "line": 12,
-            },
-            "start": Object {
-              "character": 0,
-              "line": 12,
-            },
-          },
-          "severity": 2,
-          "source": "bash-language-server",
-        },
-      ]
-    `)
+    expect(diagnostics).toEqual([])
+    expect(loggerWarn).toHaveBeenCalledWith(
+      'Error while parsing dummy-uri.sh: syntax error',
+    )
   })
 
-  it('returns a list of diagnostics for a file with parsing errors', () => {
+  it('parses a file with parsing errors', () => {
     const diagnostics = analyzer.analyze({
       uri: CURRENT_URI,
       document: FIXTURE_DOCUMENT.PARSE_PROBLEMS,
     })
-    expect(diagnostics).not.toEqual([])
-    expect(diagnostics).toMatchInlineSnapshot(`
-      Array [
-        Object {
-          "message": "Failed to parse",
-          "range": Object {
-            "end": Object {
-              "character": 1,
-              "line": 9,
-            },
-            "start": Object {
-              "character": 0,
-              "line": 2,
-            },
-          },
-          "severity": 1,
-          "source": "bash-language-server",
-        },
-      ]
-    `)
+    expect(diagnostics).toEqual([])
+    expect(loggerWarn).toHaveBeenCalledWith(
+      'Error while parsing dummy-uri.sh: syntax error',
+    )
   })
 
   it('returns a list of diagnostics for a file with sourcing issues', async () => {
@@ -802,7 +771,12 @@ describe('initiateBackgroundAnalysis', () => {
       globPattern: defaultConfig.globPattern,
     })
 
-    expect(loggerWarn).not.toHaveBeenCalled()
+    expect(loggerWarn).toHaveBeenCalled()
+    expect(loggerWarn.mock.calls).toEqual([
+      [expect.stringContaining('missing-node.sh: syntax error')],
+      [expect.stringContaining('not-a-shell-script.sh: syntax error')],
+      [expect.stringContaining('parse-problems.sh: syntax error')],
+    ])
 
     // Intro, stats on glob, one file skipped due to shebang, and outro
     expect(filesParsed).toEqual(FIXTURE_FILES_MATCHING_GLOB)

server/src/__tests__/config.test.ts

@@ -8,7 +8,6 @@ describe('ConfigSchema', () => {
         "backgroundAnalysisMaxFiles": 500,
         "explainshellEndpoint": "",
         "globPattern": "**/*@(.sh|.inc|.bash|.command)",
-        "highlightParsingErrors": false,
         "includeAllWorkspaceSymbols": false,
         "logLevel": "info",
         "shellcheckArguments": Array [],
@@ -22,7 +21,6 @@ describe('ConfigSchema', () => {
         backgroundAnalysisMaxFiles: 1,
         explainshellEndpoint: 'localhost:8080',
         globPattern: '**/*@(.sh)',
-        highlightParsingErrors: true,
         includeAllWorkspaceSymbols: true,
         shellcheckArguments: ' -e SC2001  -e SC2002 ',
         shellcheckPath: '',
@@ -32,7 +30,6 @@ describe('ConfigSchema', () => {
         "backgroundAnalysisMaxFiles": 1,
         "explainshellEndpoint": "localhost:8080",
         "globPattern": "**/*@(.sh)",
-        "highlightParsingErrors": true,
         "includeAllWorkspaceSymbols": true,
         "logLevel": "info",
         "shellcheckArguments": Array [
@@ -63,7 +60,6 @@ describe('getConfigFromEnvironmentVariables', () => {
         "backgroundAnalysisMaxFiles": 500,
         "explainshellEndpoint": "",
         "globPattern": "**/*@(.sh|.inc|.bash|.command)",
-        "highlightParsingErrors": false,
         "includeAllWorkspaceSymbols": false,
         "logLevel": "info",
         "shellcheckArguments": Array [],
@@ -82,7 +78,6 @@ describe('getConfigFromEnvironmentVariables', () => {
         "backgroundAnalysisMaxFiles": 500,
         "explainshellEndpoint": "",
         "globPattern": "**/*@(.sh|.inc|.bash|.command)",
-        "highlightParsingErrors": false,
         "includeAllWorkspaceSymbols": false,
         "logLevel": "info",
         "shellcheckArguments": Array [],
@@ -106,7 +101,6 @@ describe('getConfigFromEnvironmentVariables', () => {
         "backgroundAnalysisMaxFiles": 1,
         "explainshellEndpoint": "localhost:8080",
         "globPattern": "*.*",
-        "highlightParsingErrors": false,
         "includeAllWorkspaceSymbols": false,
         "logLevel": "error",
         "shellcheckArguments": Array [
@@ -127,27 +121,27 @@ describe('getConfigFromEnvironmentVariables', () => {
   })
   it('parses boolean environment variables', () => {
     process.env = {
-      HIGHLIGHT_PARSING_ERRORS: 'true',
+      INCLUDE_ALL_WORKSPACE_SYMBOLS: 'true',
     }
-    let result = getConfigFromEnvironmentVariables().config.highlightParsingErrors
+    let result = getConfigFromEnvironmentVariables().config.includeAllWorkspaceSymbols
     expect(result).toEqual(true)
 
     process.env = {
-      HIGHLIGHT_PARSING_ERRORS: '1',
+      INCLUDE_ALL_WORKSPACE_SYMBOLS: '1',
     }
-    result = getConfigFromEnvironmentVariables().config.highlightParsingErrors
+    result = getConfigFromEnvironmentVariables().config.includeAllWorkspaceSymbols
     expect(result).toEqual(true)
 
     process.env = {
-      HIGHLIGHT_PARSING_ERRORS: '0',
+      INCLUDE_ALL_WORKSPACE_SYMBOLS: '0',
     }
-    result = getConfigFromEnvironmentVariables().config.highlightParsingErrors
+    result = getConfigFromEnvironmentVariables().config.includeAllWorkspaceSymbols
     expect(result).toEqual(false)
 
     process.env = {
-      HIGHLIGHT_PARSING_ERRORS: 'false',
+      INCLUDE_ALL_WORKSPACE_SYMBOLS: 'false',
     }
-    result = getConfigFromEnvironmentVariables().config.highlightParsingErrors
+    result = getConfigFromEnvironmentVariables().config.includeAllWorkspaceSymbols
     expect(result).toEqual(false)
   })
 })

server/src/analyser.ts

@@ -57,9 +57,6 @@ export default class Analyzer {
   /**
    * Analyze the given document, cache the tree-sitter AST, and iterate over the
    * tree to find declarations.
-   *
-   * Returns all, if any, syntax errors that occurred while parsing the file.
-   *
    */
   public analyze({
     document,
@@ -68,11 +65,12 @@ export default class Analyzer {
     document: TextDocument
     uri: string
   }): LSP.Diagnostic[] {
+    const diagnostics: LSP.Diagnostic[] = []
     const fileContent = document.getText()
 
     const tree = this.parser.parse(fileContent)
 
-    const { diagnostics, globalDeclarations } = getGlobalDeclarations({ tree, uri })
+    const globalDeclarations = getGlobalDeclarations({ tree, uri })
 
     const sourceCommands = sourcing.getSourceCommands({
       fileUri: uri,
@@ -115,24 +113,10 @@ export default class Analyzer {
         })
     }
 
-    function findMissingNodes(node: Parser.SyntaxNode) {
-      if (node.isMissing()) {
-        diagnostics.push(
-          LSP.Diagnostic.create(
-            TreeSitterUtil.range(node),
-            `Syntax error: expected "${node.type}" somewhere in the file`,
-            LSP.DiagnosticSeverity.Warning,
-            undefined,
-            'bash-language-server',
-          ),
-        )
-      } else if (node.hasError()) {
-        node.children.forEach(findMissingNodes)
-      }
+    if (tree.rootNode.hasError()) {
+      logger.warn(`Error while parsing ${uri}: syntax error`)
     }
 
-    findMissingNodes(tree.rootNode)
-
     return diagnostics
   }
 

server/src/config.ts

@@ -16,9 +16,6 @@ export const ConfigSchema = z.object({
   // Log level for the server. To set the right log level from the start please also use the environment variable 'BASH_IDE_LOG_LEVEL'.
   logLevel: z.enum(LOG_LEVELS).default(DEFAULT_LOG_LEVEL),
 
-  // Controls if Treesitter parsing errors will be highlighted as problems.
-  highlightParsingErrors: z.boolean().default(false),
-
   // Controls how symbols (e.g. variables and functions) are included and used for completion and documentation.
   // If false, then we only include symbols from sourced files (i.e. using non dynamic statements like 'source file.sh' or '. file.sh').
   // If true, then all symbols from the workspace are included.
@@ -52,7 +49,6 @@ export function getConfigFromEnvironmentVariables(): {
     backgroundAnalysisMaxFiles: toNumber(process.env.BACKGROUND_ANALYSIS_MAX_FILES),
     explainshellEndpoint: process.env.EXPLAINSHELL_ENDPOINT,
     globPattern: process.env.GLOB_PATTERN,
-    highlightParsingErrors: toBoolean(process.env.HIGHLIGHT_PARSING_ERRORS),
     includeAllWorkspaceSymbols: toBoolean(process.env.INCLUDE_ALL_WORKSPACE_SYMBOLS),
     logLevel: process.env[LOG_LEVEL_ENV_VAR],
     shellcheckArguments: process.env.SHELLCHECK_ARGUMENTS,

server/src/server.ts

@@ -289,15 +289,8 @@ export default class BashServer {
   public async analyzeAndLintDocument(document: TextDocument) {
     const { uri } = document
 
-    let diagnostics: LSP.Diagnostic[] = []
-
     // Load the tree for the modified contents into the analyzer:
-    const analyzeDiagnostics = this.analyzer.analyze({ uri, document })
-    // Treesitter's diagnostics can be a bit inaccurate, so we only merge the
-    // analyzer's diagnostics if the setting is enabled:
-    if (this.config.highlightParsingErrors) {
-      diagnostics = diagnostics.concat(analyzeDiagnostics)
-    }
+    let diagnostics = this.analyzer.analyze({ uri, document })
 
     // Run ShellCheck diagnostics:
     if (this.linter) {

server/src/util/declarations.ts

@@ -32,38 +32,20 @@ export function getGlobalDeclarations({
 }: {
   tree: Parser.Tree
   uri: string
-}): {
-  diagnostics: LSP.Diagnostic[]
-  globalDeclarations: GlobalDeclarations
-} {
-  const diagnostics: LSP.Diagnostic[] = []
+}): GlobalDeclarations {
   const globalDeclarations: GlobalDeclarations = {}
 
   tree.rootNode.children.forEach((node) => {
-    if (node.type === 'ERROR') {
-      diagnostics.push(
-        LSP.Diagnostic.create(
-          TreeSitterUtil.range(node),
-          'Failed to parse',
-          LSP.DiagnosticSeverity.Error,
-          undefined,
-          'bash-language-server',
-        ),
-      )
-      return
-    }
-
     if (TreeSitterUtil.isDefinition(node)) {
       const symbol = nodeToSymbolInformation({ node, uri })
-
       if (symbol) {
         const word = symbol.name
         globalDeclarations[word] = symbol
       }
     }
   })
 
-  return { diagnostics, globalDeclarations }
+  return globalDeclarations
 }
 
 /**

vscode-client/package.json

@@ -47,11 +47,6 @@
           "default": "**/*@(.sh|.inc|.bash|.command)",
           "description": "Glob pattern for finding and parsing shell script files in the workspace. Used by the background analysis features across files."
         },
-        "bashIde.highlightParsingErrors": {
-          "type": "boolean",
-          "default": false,
-          "description": "Controls if Treesitter parsing errors will be highlighted as problems."
-        },
         "bashIde.includeAllWorkspaceSymbols": {
           "type": "boolean",
           "default": false,