fix(diagnostics): refresh for empty collection when displayByAle

BREAKING CHANGE: remove refreshAfterSave support.

Closes #2726
Closes #2731

data/schema.json

@@ -542,11 +542,6 @@
       "description": "Enable diagnostic refresh on insert mode, default false.",
       "default": false
     },
-    "diagnostic.refreshAfterSave": {
-      "type": "boolean",
-      "description": "Only refresh diagnostics after save, default false.",
-      "default": false
-    },
     "diagnostic.displayByAle": {
       "type": "boolean",
       "description": "Use Ale for display diagnostics in vim, will disable coc for display diagnostics, restart required on change.",

doc/coc.cnx

@@ -362,10 +362,6 @@ Built-in configurations:~
 
 	Refresh diagnostics when in insert mode, default: `false`
 
-"diagnostic.refreshAfterSave":~
-
-	Refresh diagnostics after file save only, default: `false`
-
 "diagnostic.displayByAle":~
 
 	Use ALE for displaying diagnostics. This will disable coc.nvim for

doc/coc.txt

@@ -389,10 +389,6 @@ Built-in configurations:~
 
 	Refresh diagnostics when in insert mode, default: `false`
 
-"diagnostic.refreshAfterSave":~
-
-	Refresh diagnostics after file save only, default: `false`
-
 "diagnostic.displayByAle":~
 
 	Use ALE for displaying diagnostics. This will disable coc.nvim for

src/__tests__/modules/diagnosticBuffer.test.ts

@@ -28,7 +28,6 @@ const config: DiagnosticConfig = {
   errorSign: '>>',
   warningSign: '>>',
   infoSign: '>>',
-  refreshAfterSave: false,
   hintSign: '>>',
   filetypeMap: {
     default: ''

src/__tests__/modules/diagnosticManager.test.ts

@@ -7,7 +7,7 @@ import Document from '../../model/document'
 import workspace from '../../workspace'
 import window from '../../window'
 import manager from '../../diagnostic/manager'
-import helper from '../helper'
+import helper, { createTmpFile } from '../helper'
 
 let nvim: Neovim
 function createDiagnostic(msg: string, range?: Range, severity?: DiagnosticSeverity): Diagnostic {
@@ -258,4 +258,27 @@ describe('diagnostic manager', () => {
     expect(diagnostics[0].message).toBe('error')
     collection.dispose()
   })
+
+  it('should send ale diagnostic items', async () => {
+    let config = workspace.getConfiguration('diagnostic')
+    config.update('displayByAle', true)
+    let content = `
+    function! MockAleResults(bufnr, collection, items)
+      let g:collection = a:collection
+      let g:items = a:items
+    endfunction
+    `
+    let file = await createTmpFile(content)
+    await nvim.command(`source ${file}`)
+    await createDocument()
+    let items = await nvim.getVar('items') as any[]
+    expect(Array.isArray(items)).toBe(true)
+    expect(items.length).toBeGreaterThan(0)
+    let collection = manager.getCollectionByName('test')
+    collection.clear()
+    await helper.wait(100)
+    items = await nvim.getVar('items') as any[]
+    expect(items).toEqual([])
+    config.update('displayByAle', false)
+  })
 })

src/diagnostic/manager.ts

@@ -36,7 +36,6 @@ export interface DiagnosticConfig {
   messageDelay: number
   maxWindowHeight: number
   maxWindowWidth: number
-  refreshAfterSave: boolean
   refreshOnInsertMode: boolean
   virtualText: boolean
   virtualTextCurrentLineOnly: boolean
@@ -102,8 +101,7 @@ export class DiagnosticManager implements Disposable {
       let doc = workspace.getDocument(bufnr)
       if (!doc) return
       doc.forceSync()
-      let { refreshOnInsertMode, refreshAfterSave } = this.config
-      if (!refreshOnInsertMode && !refreshAfterSave) {
+      if (!this.config.refreshOnInsertMode) {
         this.refreshBuffer(doc.uri)
       }
     }, null, this.disposables)
@@ -112,13 +110,6 @@ export class DiagnosticManager implements Disposable {
       if (this.timer) clearTimeout(this.timer)
     }, null, this.disposables)
 
-    events.on('BufWritePost', async bufnr => {
-      let buf = this.buffers.get(bufnr)
-      if (!buf) return
-      if (!this.config.refreshAfterSave) return
-      this.refreshBuffer(buf.uri)
-    }, null, this.disposables)
-
     workspace.onDidChangeConfiguration(e => {
       this.setConfiguration(e)
     }, null, this.disposables)
@@ -216,16 +207,11 @@ export class DiagnosticManager implements Disposable {
    * Create collection by name
    */
   public create(name: string): DiagnosticCollection {
-    let collection = new DiagnosticCollection(name)
+    let collection = this.getCollectionByName(name)
+    if (collection) return collection
+    collection = new DiagnosticCollection(name)
     this.collections.push(collection)
-    // Used for refresh diagnostics on buferEnter when refreshAfterSave is true
-    // Note we can't make sure it work as expected when there're multiple sources
-    let createTime = Date.now()
-    let refreshed = false
     collection.onDidDiagnosticsChange(uri => {
-      if (this.config.refreshAfterSave &&
-        (refreshed || Date.now() - createTime > 5000)) return
-      refreshed = true
       this.refreshBuffer(uri)
     })
     collection.onDidDiagnosticsClear(uris => {
@@ -621,7 +607,6 @@ export class DiagnosticManager implements Disposable {
       warningSign: config.get<string>('warningSign', '>>'),
       infoSign: config.get<string>('infoSign', '>>'),
       hintSign: config.get<string>('hintSign', '>>'),
-      refreshAfterSave: config.get<boolean>('refreshAfterSave', false),
       refreshOnInsertMode: config.get<boolean>('refreshOnInsertMode', false),
       filetypeMap: config.get<object>('filetypeMap', {}),
       showUnused: config.get<boolean>('showUnused', true),
@@ -679,28 +664,38 @@ export class DiagnosticManager implements Disposable {
   }
 
   public refreshBuffer(uri: string, force = false): boolean {
-    let buf = Array.from(this.buffers.values()).find(o => o.uri == uri)
-    if (!buf || !this.enabled) return false
-    let { displayByAle, refreshOnInsertMode } = this.config
-    if (!refreshOnInsertMode && workspace.insertMode) return false
+    if (!this.enabled) return false
+    let { displayByAle } = this.config
     let diagnostics = this.getDiagnostics(uri)
     if (!displayByAle) {
+      let buf = Array.from(this.buffers.values()).find(o => o.uri == uri)
+      if (!buf) return false
       if (force) {
         buf.forceRefresh(diagnostics)
       } else {
         buf.refresh(diagnostics)
       }
       return true
     } else {
+      let doc = workspace.getDocument(uri)
+      if (!doc) return
+      if (!this.config.refreshOnInsertMode && workspace.insertMode) return false
       let exists = this.aleDiagnosticsMap.get(uri) || []
       if (equals(diagnostics, exists)) return false
       this.aleDiagnosticsMap.set(uri, diagnostics)
       let map: Map<string, Diagnostic[]> = new Map()
+      let collections = new Set(exists.map(o => o.collection))
       diagnostics.forEach(o => {
         let exists = map.get(o.collection) || []
         exists.push(o)
         map.set(o.collection, exists)
       })
+      // clear old collection.
+      for (let name of collections) {
+        if (!map.has(name)) {
+          map.set(name, [])
+        }
+      }
       this.nvim.pauseNotification()
       for (let [collection, diagnostics] of map.entries()) {
         let aleItems = diagnostics.map(o => {
@@ -715,7 +710,8 @@ export class DiagnosticManager implements Disposable {
             type: getSeverityType(o.severity)
           }
         })
-        this.nvim.call('ale#other_source#ShowResults', [buf.bufnr, collection, aleItems], true)
+        let method = global.hasOwnProperty('__TEST__') ? 'MockAleResults' : 'ale#other_source#ShowResults'
+        this.nvim.call(method, [doc.bufnr, collection, aleItems], true)
       }
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
       this.nvim.resumeNotification(false, true)