fix(diagnostic): convert to byte index

commit 7b13d6670c0cb1e98383b1371733b74d8f128555 Author: Qiming Zhao <[email protected]> Date: Mon May 2 17:20:13 2022 +0800 refactor diagnostic locationlist related commit aec6eda Author: kevinhwang91 <[email protected]> Date: Tue Apr 26 14:56:59 2022 +0800 skip empty diagnostic commit ecca507 Author: kevinhwang91 <[email protected]> Date: Tue Apr 26 12:17:12 2022 +0800 fix(diagnostic): convert to byte index
neoclide · May 2, 2022 · 32cad71 · 32cad71
1 parent f32eba4
commit 32cad71
Show file tree

Hide file tree

Showing 9 changed files with 98 additions and 59 deletions.
diff --git a/src/__tests__/modules/diagnosticBuffer.test.ts b/src/__tests__/modules/diagnosticBuffer.test.ts
@@ -35,7 +35,7 @@ const config: any = {
 
 async function createDiagnosticBuffer(): Promise<DiagnosticBuffer> {
   let doc = await workspace.document
-  return new DiagnosticBuffer(nvim, doc.bufnr, doc.uri, config, () => {
+  return new DiagnosticBuffer(nvim, doc, config, () => {
     // noop
   })
 }

diff --git a/src/__tests__/modules/diagnosticManager.test.ts b/src/__tests__/modules/diagnosticManager.test.ts
@@ -61,6 +61,24 @@ describe('diagnostic manager', () => {
       res = await nvim.call('getloclist', [doc.bufnr]) as any[]
       expect(res.length).toBe(2)
     })
+
+    it('should throw when diagnostic disabled', async () => {
+      helper.updateConfiguration('diagnostic.enable', false)
+      let fn = async () => {
+        let bufnr = await nvim.call('bufnr', ['%'])
+        await manager.setLocationlist(bufnr)
+      }
+      await expect(fn()).rejects.toThrow(/not enabled/)
+    })
+
+    it('should throw when buffer not attached', async () => {
+      await nvim.command(`vnew +setl\\ buftype=nofile`)
+      let doc = await workspace.document
+      let fn = async () => {
+        await manager.setLocationlist(doc.bufnr)
+      }
+      await expect(fn()).rejects.toThrow(/not/)
+    })
   })
 
   describe('events', () => {
@@ -180,7 +198,7 @@ describe('diagnostic manager', () => {
       diagnostics.push(createDiagnostic('error', Range.create(0, 0, 0, 1), DiagnosticSeverity.Error))
       diagnostics.push(createDiagnostic('error', Range.create(0, 2, 0, 3), DiagnosticSeverity.Warning))
       collection.set(doc.uri, diagnostics)
-      let list = manager.getDiagnosticList()
+      let list = await manager.getDiagnosticList()
       expect(list).toBeDefined()
       expect(list.length).toBeGreaterThanOrEqual(5)
       expect(list[0].severity).toBe('Error')
@@ -197,7 +215,7 @@ describe('diagnostic manager', () => {
       let diagnostics = manager.getDiagnostics(doc.uri)['test']
       diagnostics[0].tags = [DiagnosticTag.Unnecessary]
       diagnostics[2].tags = [DiagnosticTag.Deprecated]
-      let list = manager.getDiagnosticList()
+      let list = await manager.getDiagnosticList()
       expect(list.length).toBe(3)
       let res = manager.getDiagnostics(doc.uri)['test']
       expect(res.length).toBe(1)
@@ -495,11 +513,10 @@ describe('diagnostic manager', () => {
       await nvim.call('cursor', [1, 2])
       await manager.echoMessage(false)
       let win = await helper.getFloat()
-      await nvim.call('win_gotoid', [win.id])
-      await helper.wait(50)
-      let res = await nvim.eval('synIDattr(synID(1,13,1),"name")')
-      expect(res).toMatch(/javascript/i)
-      await nvim.command('q')
+      let bufnr = await nvim.call('winbufnr', [win.id])
+      let buf = nvim.createBuffer(bufnr)
+      let lines = await buf.lines
+      expect(lines.join('\n')).toMatch('www.example.com')
     })
 
     it('should show floating window on cursor hold', async () => {

diff --git a/src/core/documents.ts b/src/core/documents.ts
@@ -503,17 +503,19 @@ export default class Documents implements Disposable {
     }
     let doc = this.getDocument(loc.uri)
     let { uri, range } = loc
+    let { start, end } = range
     let u = URI.parse(uri)
     if (!text && u.scheme == 'file') {
-      text = await this.getLine(uri, range.start.line)
+      text = await this.getLine(uri, start.line)
     }
+    let endLine = start.line == end.line ? text : await this.getLine(uri, end.line)
     let item: QuickfixItem = {
       uri,
       filename: u.scheme == 'file' ? u.fsPath : uri,
-      lnum: range.start.line + 1,
-      end_lnum: range.end.line + 1,
-      col: text ? byteIndex(text, range.start.character) + 1 : range.start.character + 1,
-      end_col: text ? byteIndex(text, range.end.character) + 1 : range.end.character + 1,
+      lnum: start.line + 1,
+      end_lnum: end.line + 1,
+      col: text ? byteIndex(text, start.character) + 1 : start.character + 1,
+      end_col: endLine ? byteIndex(endLine, end.character) + 1 : end.character + 1,
       text: text || '',
       range
     }
@@ -528,7 +530,7 @@ export default class Documents implements Disposable {
    */
   public async getLine(uri: string, line: number): Promise<string> {
     let document = this.getDocument(uri)
-    if (document) return document.getline(line) || ''
+    if (document && document.attached) return document.getline(line) || ''
     if (!uri.startsWith('file:')) return ''
     let fsPath = URI.parse(uri).fsPath
     if (!fs.existsSync(fsPath)) return ''

diff --git a/src/diagnostic/buffer.ts b/src/diagnostic/buffer.ts
@@ -7,6 +7,7 @@ import { SyncItem } from '../model/bufferSync'
 import { DidChangeTextDocumentParams, HighlightItem, LocationListItem } from '../types'
 import { equals } from '../util/object'
 import { lineInRange, positionInRange } from '../util/position'
+import Document from '../model/document'
 import workspace from '../workspace'
 import { DiagnosticConfig, getHighlightGroup, getLocationListItem, getNameFromSeverity, getSeverityType, sortDiagnostics } from './util'
 const logger = require('../util/logger')('diagnostic-buffer')
@@ -45,8 +46,7 @@ export class DiagnosticBuffer implements SyncItem {
   public refreshHighlights: Function & { clear(): void }
   constructor(
     private readonly nvim: Neovim,
-    public readonly bufnr: number,
-    public readonly uri: string,
+    public readonly doc: Document,
     private config: DiagnosticConfig,
     private onRefresh: (diagnostics: ReadonlyArray<Diagnostic>) => void
   ) {
@@ -58,6 +58,14 @@ export class DiagnosticBuffer implements SyncItem {
     return this._dirty
   }
 
+  public get bufnr(): number {
+    return this.doc.bufnr
+  }
+
+  public get uri(): string {
+    return this.doc.uri
+  }
+
   public onChange(e: DidChangeTextDocumentParams): void {
     this._changeTs = Date.now()
     this.refreshHighlights.clear()
@@ -74,11 +82,6 @@ export class DiagnosticBuffer implements SyncItem {
     return this.config.displayByAle
   }
 
-  private get changedTick(): number {
-    let doc = workspace.getDocument(this.uri)
-    return doc ? doc.changedtick : 0
-  }
-
   private clearHighlight(collection: string): void {
     this.buffer.clearNamespace(NAMESPACE + collection)
   }
@@ -128,7 +131,7 @@ export class DiagnosticBuffer implements SyncItem {
     diagnosticsMap.set(collection, diagnostics)
     // avoid refresh when no change happened between previous refresh
     if (this._dirty === false
-      && this.changedTick == this._changedTick
+      && this.doc.changedtick == this._changedTick
       && equals(curr, diagnostics)) {
       return
     }
@@ -196,7 +199,7 @@ export class DiagnosticBuffer implements SyncItem {
   private refresh(diagnosticsMap: Map<string, ReadonlyArray<Diagnostic>>, info: DiagnosticInfo): void {
     let { nvim, displayByAle } = this
     this._dirty = false
-    this._changedTick = this.changedTick
+    this._changedTick = this.doc.changedtick
     if (displayByAle) {
       nvim.pauseNotification()
       for (let [collection, diagnostics] of diagnosticsMap.entries()) {
@@ -226,14 +229,20 @@ export class DiagnosticBuffer implements SyncItem {
 
   public updateLocationList(winid: number, title: string): void {
     if (!this.config.locationlistUpdate || winid == -1 || title !== 'Diagnostics of coc') return
+    let items = this.toLocationListItems(this.diagnostics)
+    this.nvim.call('setloclist', [winid, [], 'r', { title: 'Diagnostics of coc', items }], true)
+  }
+
+  public toLocationListItems(diagnostics: Diagnostic[]): LocationListItem[] {
+    let { locationlistLevel } = this.config
     let items: LocationListItem[] = []
-    let { diagnostics } = this
+    let lines = this.doc.textDocument.lines
     diagnostics.sort(sortDiagnostics)
     for (let diagnostic of diagnostics) {
-      let item = getLocationListItem(this.bufnr, diagnostic)
-      items.push(item)
+      if (locationlistLevel && diagnostic.severity && diagnostic.severity > locationlistLevel) continue
+      items.push(getLocationListItem(this.bufnr, diagnostic, lines))
     }
-    this.nvim.call('setloclist', [winid, [], 'r', { title: 'Diagnostics of coc', items }], true)
+    return items
   }
 
   public addSigns(collection: string, diagnostics: ReadonlyArray<Diagnostic>): void {

diff --git a/src/diagnostic/collection.ts b/src/diagnostic/collection.ts
@@ -75,6 +75,10 @@ export default class DiagnosticCollection {
     }
   }
 
+  public entries(): IterableIterator<[string, Diagnostic[]]> {
+    return this.diagnosticsMap.entries()
+  }
+
   public get(uri: string): Diagnostic[] {
     let arr = this.diagnosticsMap.get(uri)
     return arr == null ? [] : arr.slice()

diff --git a/src/diagnostic/manager.ts b/src/diagnostic/manager.ts
@@ -7,15 +7,16 @@ import { URI } from 'vscode-uri'
 import events from '../events'
 import BufferSync from '../model/bufferSync'
 import FloatFactory from '../model/floatFactory'
-import { ConfigurationChangeEvent, Documentation, ErrorItem, LocationListItem } from '../types'
+import { ConfigurationChangeEvent, Documentation, ErrorItem } from '../types'
 import { disposeAll } from '../util'
+import { readFileLines } from '../util/fs'
 import { comparePosition, rangeIntersect } from '../util/position'
-import { characterIndex } from '../util/string'
+import { byteIndex, characterIndex } from '../util/string'
 import window from '../window'
 import workspace from '../workspace'
 import { DiagnosticBuffer } from './buffer'
 import DiagnosticCollection from './collection'
-import { DiagnosticConfig, getLocationListItem, getSeverityName, severityLevel } from './util'
+import { DiagnosticConfig, getSeverityName, severityLevel } from './util'
 const logger = require('../util/logger')('diagnostic-manager')
 
 export interface DiagnosticEventParams {
@@ -61,9 +62,9 @@ export class DiagnosticManager implements Disposable {
 
     this.floatFactory = new FloatFactory(this.nvim)
     this.buffers = workspace.registerBufferSync(doc => {
-      if (doc.buftype !== '') return undefined
+      if (!this.enabled) return undefined
       let buf = new DiagnosticBuffer(
-        this.nvim, doc.bufnr, doc.uri, this.config,
+        this.nvim, doc, this.config,
         diagnostics => {
           this._onDidRefresh.fire({ diagnostics, uri: buf.uri, bufnr: buf.bufnr })
           if (buf.bufnr === workspace.bufnr && this.config.messageTarget === 'float') {
@@ -72,7 +73,6 @@ export class DiagnosticManager implements Disposable {
             })
           }
         })
-      if (!this.enabled) return
       let diagnostics = this.getDiagnostics(doc.uri)
       // ignore empty diagnostics on first time.
       if (Object.keys(diagnostics).length > 0) {
@@ -164,18 +164,15 @@ export class DiagnosticManager implements Disposable {
    * Fill location list with diagnostics
    */
   public async setLocationlist(bufnr: number): Promise<void> {
-    let { locationlistLevel } = this.config
+    if (!this.enabled) throw new Error(`Diagnostic not enabled.`)
     let buf = this.buffers.getItem(bufnr)
-    let diagnosticsMap = buf ? this.getDiagnostics(buf.uri) : {}
-    let items: LocationListItem[] = []
-    for (let diagnostics of Object.values(diagnosticsMap)) {
-      for (let diagnostic of diagnostics) {
-        if (locationlistLevel && diagnostic.severity && diagnostic.severity > locationlistLevel) continue
-        let item = getLocationListItem(bufnr, diagnostic)
-        items.push(item)
-      }
+    if (!buf) throw new Error(`buffer ${bufnr} not attached.`)
+    let diagnostics: Diagnostic[] = []
+    for (let diags of Object.values(this.getDiagnostics(buf.uri))) {
+      diagnostics.push(...diags)
     }
-    let curr = await this.nvim.call('getloclist', [0, { title: 1 }]) as any
+    let items = buf.toLocationListItems(diagnostics)
+    let curr = await this.nvim.call('getloclist', [0, { title: 1 }]) as { title?: string }
     let action = curr.title && curr.title.indexOf('Diagnostics of coc') != -1 ? 'r' : ' '
     await this.nvim.call('setloclist', [0, [], action, { title: 'Diagnostics of coc', items }])
   }
@@ -377,25 +374,34 @@ export class DiagnosticManager implements Disposable {
   }
 
   /**
-   * Get all sorted diagnostics of current workspace
+   * Get all sorted diagnostics
    */
-  public getDiagnosticList(): DiagnosticItem[] {
+  public async getDiagnosticList(): Promise<DiagnosticItem[]> {
     let res: DiagnosticItem[] = []
     const { level } = this.config
     for (let collection of this.collections) {
-      collection.forEach((uri, diagnostics) => {
-        let file = URI.parse(uri).fsPath
+      for (let [uri, diagnostics] of collection.entries()) {
+        if (diagnostics.length == 0) continue
+        let u = URI.parse(uri)
+        let doc = workspace.getDocument(uri)
+        let lines = doc && doc.attached ? doc.textDocument.lines : undefined
+        if (!lines && u.scheme === 'file') {
+          try {
+            const max = diagnostics.reduce((p, c) => {
+              return Math.max(c.range.end.line, p)
+            }, 0)
+            lines = await readFileLines(u.fsPath, 0, max)
+          } catch (e) {}
+        }
         for (let diagnostic of diagnostics) {
-          if (diagnostic.severity && diagnostic.severity > level) {
-            continue
-          }
+          if (diagnostic.severity && diagnostic.severity > level) continue
           let { start, end } = diagnostic.range
           let o: DiagnosticItem = {
-            file,
+            file: u.fsPath,
             lnum: start.line + 1,
             end_lnum: end.line + 1,
-            col: start.character + 1,
-            end_col: end.character + 1,
+            col: Array.isArray(lines) ? byteIndex(lines[start.line] ?? '', start.character) + 1 : start.character + 1,
+            end_col: Array.isArray(lines) ? byteIndex(lines[end.line] ?? '', end.character) + 1 : end.character + 1,
             code: diagnostic.code,
             source: diagnostic.source || collection.name,
             message: diagnostic.message,
@@ -405,7 +411,7 @@ export class DiagnosticManager implements Disposable {
           }
           res.push(o)
         }
-      })
+      }
     }
     res.sort((a, b) => {
       if (a.level !== b.level) {

diff --git a/src/diagnostic/util.ts b/src/diagnostic/util.ts
@@ -2,6 +2,7 @@
 import { DiagnosticSeverity, Diagnostic, DiagnosticTag } from 'vscode-languageserver-protocol'
 import { FloatConfig, LocationListItem } from '../types'
 import { comparePosition } from '../util/position'
+import { byteIndex } from '../util/string'
 
 export enum DiagnosticHighlight {
   Error = 'CocErrorHighlight',
@@ -107,7 +108,7 @@ export function getNameFromSeverity(severity: DiagnosticSeverity): string {
   }
 }
 
-export function getLocationListItem(bufnr: number, diagnostic: Diagnostic): LocationListItem {
+export function getLocationListItem(bufnr: number, diagnostic: Diagnostic, lines?: ReadonlyArray<string>): LocationListItem {
   let { start, end } = diagnostic.range
   let owner = diagnostic.source || 'coc.nvim'
   let msg = diagnostic.message.split('\n')[0]
@@ -116,8 +117,8 @@ export function getLocationListItem(bufnr: number, diagnostic: Diagnostic): Loca
     bufnr,
     lnum: start.line + 1,
     end_lnum: end.line + 1,
-    col: start.character + 1,
-    end_col: end.character + 1,
+    col: Array.isArray(lines) ? byteIndex(lines[start.line] ?? '', start.character) + 1 : start.character + 1,
+    end_col: Array.isArray(lines) ? byteIndex(lines[end.line] ?? '', end.character) + 1 : end.character + 1,
     text: `[${owner}${diagnostic.code ? ' ' + diagnostic.code : ''}] ${msg} [${type}]`,
     type
   }

diff --git a/src/list/source/diagnostics.ts b/src/list/source/diagnostics.ts
@@ -22,7 +22,7 @@ export default class DiagnosticsList extends LocationList {
   }
 
   public async loadItems(context: ListContext): Promise<ListItem[]> {
-    let list = diagnosticManager.getDiagnosticList()
+    let list = await diagnosticManager.getDiagnosticList()
     let { cwd } = context
     const config = this.getConfig()
     const shouldIncludeCode = config.get<boolean>('includeCode', true)

diff --git a/src/plugin.ts b/src/plugin.ts
@@ -103,7 +103,7 @@ export default class Plugin extends EventEmitter {
     this.addAction('diagnosticNext', severity => diagnosticManager.jumpNext(severity))
     this.addAction('diagnosticPrevious', severity => diagnosticManager.jumpPrevious(severity))
     this.addAction('diagnosticPreview', () => diagnosticManager.preview())
-    this.addAction('diagnosticList', () => diagnosticManager.getDiagnosticList())
+    this.addAction('diagnosticList', async () => diagnosticManager.getDiagnosticList())
     this.addAction('findLocations', (id, method, params, openCommand) => this.handler.locations.findLocations(id, method, params, openCommand))
     this.addAction('getTagList', () => this.handler.locations.getTagList())
     this.addAction('jumpDefinition', openCommand => this.handler.locations.gotoDefinition(openCommand))