Merge pull request #18375 from atom/mb-highlighting-on-fast-edits

Fix error causing tree-sitter highlighting to get out-of-sync when editing rapidly

package-lock.json

@@ -5529,9 +5529,9 @@
       "integrity": "sha512-DlX6dR0lOIRDFxI0mjL9IYg6OTncLm/Zt+JiBhE5OlFcAR8yc9S7FFXU9so0oda47frdM/JFsk7UjNt9vscKcg=="
     },
     "tree-sitter": {
-      "version": "0.13.15",
-      "resolved": "https://registry.npmjs.org/tree-sitter/-/tree-sitter-0.13.15.tgz",
-      "integrity": "sha512-FuvU+csO7t/rQqLdL3+w4Jg+4Zl22Y4uCi4L9X/qJG57Zn71ZzP3oHtDSRgpiIms6g3Y7cEJvF7K/rCw11q92Q==",
+      "version": "0.13.16",
+      "resolved": "https://registry.npmjs.org/tree-sitter/-/tree-sitter-0.13.16.tgz",
+      "integrity": "sha512-VWPg7cbqEk3vMM6ehAlGKRx44P2FQZgOjvObHTowM7uwI7Z2K+TF2GBI65JwYRTiRkOfbPEIRLDLA2oPQfvDMA==",
       "requires": {
         "nan": "^2.10.0",
         "prebuild-install": "^5.0.0"
@@ -5543,15 +5543,16 @@
           "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ="
         },
         "prebuild-install": {
-          "version": "5.1.0",
-          "resolved": "https://registry.npmjs.org/prebuild-install/-/prebuild-install-5.1.0.tgz",
-          "integrity": "sha512-jGdh2Ws5OUCvBm+aQ/je7hgOBfLIFcgnF9DZ1PIEvht0JKfMwn3Gy0MPHL16JcAUI6tu7LX0D3VxmvMm1XZwAw==",
+          "version": "5.2.1",
+          "resolved": "https://registry.npmjs.org/prebuild-install/-/prebuild-install-5.2.1.tgz",
+          "integrity": "sha512-9DAccsInWHB48TBQi2eJkLPE049JuAI6FjIH0oIrij4bpDVEbX6JvlWRAcAAlUqBHhjgq0jNqA3m3bBXWm9v6w==",
           "requires": {
             "detect-libc": "^1.0.3",
             "expand-template": "^1.0.2",
             "github-from-package": "0.0.0",
             "minimist": "^1.2.0",
             "mkdirp": "^0.5.1",
+            "napi-build-utils": "^1.0.1",
             "node-abi": "^2.2.0",
             "noop-logger": "^0.1.1",
             "npmlog": "^4.0.1",

package.json

@@ -159,7 +159,7 @@
     "temp": "^0.8.3",
     "text-buffer": "13.14.10",
     "timecop": "https://www.atom.io/api/packages/timecop/versions/0.36.2/tarball",
-    "tree-sitter": "0.13.15",
+    "tree-sitter": "0.13.16",
     "tree-sitter-css": "^0.13.7",
     "tree-view": "https://www.atom.io/api/packages/tree-view/versions/0.224.2/tarball",
     "typescript-simple": "1.0.0",

spec/tree-sitter-language-mode-spec.js

@@ -1,11 +1,15 @@
 const {it, fit, ffit, fffit, beforeEach, afterEach} = require('./async-spec-helpers')
 
+const fs = require('fs')
+const path = require('path')
 const dedent = require('dedent')
 const TextBuffer = require('text-buffer')
 const {Point} = TextBuffer
 const TextEditor = require('../src/text-editor')
 const TreeSitterGrammar = require('../src/tree-sitter-grammar')
 const TreeSitterLanguageMode = require('../src/tree-sitter-language-mode')
+const Random = require('../script/node_modules/random-seed')
+const {getRandomBufferRange, buildRandomLines} = require('./helpers/random')
 
 const cGrammarPath = require.resolve('language-c/grammars/tree-sitter-c.cson')
 const pythonGrammarPath = require.resolve('language-python/grammars/tree-sitter-python.cson')
@@ -789,6 +793,97 @@ describe('TreeSitterLanguageMode', () => {
     })
   })
 
+  describe('highlighting after random changes', () => {
+    let originalTimeout
+
+    beforeEach(() => {
+      originalTimeout = jasmine.getEnv().defaultTimeoutInterval
+      jasmine.getEnv().defaultTimeoutInterval = 60 * 1000
+    })
+
+    afterEach(() => {
+      jasmine.getEnv().defaultTimeoutInterval = originalTimeout
+    })
+
+    it('matches the highlighting of a freshly-opened editor', async () => {
+      jasmine.useRealClock()
+
+      const text = fs.readFileSync(path.join(__dirname, 'fixtures', 'sample.js'), 'utf8')
+      atom.grammars.loadGrammarSync(jsGrammarPath)
+      atom.grammars.assignLanguageMode(buffer, 'source.js')
+      buffer.getLanguageMode().syncOperationLimit = 0
+
+      const initialSeed = Date.now()
+      for (let i = 0, trial_count = 10; i < trial_count; i++) {
+        let seed = initialSeed + i
+        // seed = 1541201470759
+        const random = Random(seed)
+
+        // Parse the initial content and render all of the screen lines.
+        buffer.setText(text)
+        buffer.clearUndoStack()
+        await buffer.getLanguageMode().parseCompletePromise()
+        editor.displayLayer.getScreenLines()
+
+        // Make several random edits.
+        for (let j = 0, edit_count = 1 + random(4); j < edit_count; j++) {
+          const editRoll = random(10)
+          const range = getRandomBufferRange(random, buffer)
+
+          if (editRoll < 2) {
+            const linesToInsert = buildRandomLines(random, range.getExtent().row + 1)
+            // console.log('replace', range.toString(), JSON.stringify(linesToInsert))
+            buffer.setTextInRange(range, linesToInsert)
+          } else if (editRoll < 5) {
+            // console.log('delete', range.toString())
+            buffer.delete(range)
+          } else {
+            const linesToInsert = buildRandomLines(random, 3)
+            // console.log('insert', range.start.toString(), JSON.stringify(linesToInsert))
+            buffer.insert(range.start, linesToInsert)
+          }
+
+          // console.log(buffer.getText())
+
+          // Sometimes, let the parse complete before re-rendering.
+          // Sometimes re-render and move on before the parse completes.
+          if (random(2)) await buffer.getLanguageMode().parseCompletePromise()
+          editor.displayLayer.getScreenLines()
+        }
+
+        // Revert the edits, because Tree-sitter's error recovery is somewhat path-dependent,
+        // and we want a state where the tree parse result is guaranteed.
+        while (buffer.undo()) {}
+
+        // Create a fresh buffer and editor with the same text.
+        const buffer2 = new TextBuffer(buffer.getText())
+        const editor2 = new TextEditor({buffer: buffer2})
+        atom.grammars.assignLanguageMode(buffer2, 'source.js')
+
+        // Verify that the the two buffers have the same syntax highlighting.
+        await buffer.getLanguageMode().parseCompletePromise()
+        await buffer2.getLanguageMode().parseCompletePromise()
+        expect(buffer.getLanguageMode().tree.rootNode.toString()).toEqual(
+          buffer2.getLanguageMode().tree.rootNode.toString(), `Seed: ${seed}`
+        )
+
+        for (let j = 0, n = editor.getScreenLineCount(); j < n; j++) {
+          const tokens1 = editor.tokensForScreenRow(j)
+          const tokens2 = editor2.tokensForScreenRow(j)
+          expect(tokens1).toEqual(tokens2, `Seed: ${seed}, screen line: ${j}`)
+          if (jasmine.getEnv().currentSpec.results().failedCount > 0) {
+            console.log(tokens1)
+            console.log(tokens2)
+            debugger
+            break
+          }
+        }
+
+        if (jasmine.getEnv().currentSpec.results().failedCount > 0) break
+      }
+    })
+  })
+
   describe('folding', () => {
     it('can fold nodes that start and end with specified tokens', async () => {
       const grammar = new TreeSitterGrammar(atom.grammars, jsGrammarPath, {

src/tree-sitter-language-mode.js

@@ -45,7 +45,6 @@ class TreeSitterLanguageMode {
     this.hasQueuedParse = false
 
     this.grammarForLanguageString = this.grammarForLanguageString.bind(this)
-    this.emitRangeUpdate = this.emitRangeUpdate.bind(this)
 
     this.subscription = this.buffer.onDidChangeText(({changes}) => {
       for (let i = 0, {length} = changes; i < length; i++) {
@@ -70,6 +69,25 @@ class TreeSitterLanguageMode {
     this.regexesByPattern = {}
   }
 
+  async parseCompletePromise () {
+    let done = false
+    while (!done) {
+      if (this.rootLanguageLayer.currentParsePromise) {
+        await this.rootLanguageLayer.currentParsePromises
+      } else {
+        done = true
+        for (const marker of this.injectionsMarkerLayer.getMarkers()) {
+          if (marker.languageLayer.currentParsePromise) {
+            done = false
+            await marker.languageLayer.currentParsePromise
+            break
+          }
+        }
+      }
+      await new Promise(resolve => setTimeout(resolve, 0))
+    }
+  }
+
   destroy () {
     this.injectionsMarkerLayer.destroy()
     this.subscription.dispose()
@@ -548,8 +566,8 @@ class LanguageLayer {
     if (this.patchSinceCurrentParseStarted) {
       this.patchSinceCurrentParseStarted.splice(
         oldRange.start,
-        oldRange.end,
-        newRange.end,
+        oldRange.getExtent(),
+        newRange.getExtent(),
         oldText,
         newText
       )
@@ -613,10 +631,14 @@ class LanguageLayer {
 
     const changes = this.patchSinceCurrentParseStarted.getChanges()
     this.patchSinceCurrentParseStarted = null
-    for (let i = changes.length - 1; i >= 0; i--) {
-      const {oldStart, oldEnd, newEnd, oldText, newText} = changes[i]
+    for (const {oldStart, newStart, oldEnd, newEnd, oldText, newText} of changes) {
+      const newExtent = Point.fromObject(newEnd).traversalFrom(newStart)
       tree.edit(this._treeEditForBufferChange(
-        oldStart, oldEnd, newEnd, oldText, newText
+        newStart,
+        oldEnd,
+        Point.fromObject(oldStart).traverse(newExtent),
+        oldText,
+        newText
       ))
     }
 
@@ -655,9 +677,7 @@ class LanguageLayer {
   }
 
   _populateInjections (range, nodeRangeSet) {
-    const {injectionsMarkerLayer, grammarForLanguageString} = this.languageMode
-
-    const existingInjectionMarkers = injectionsMarkerLayer
+    const existingInjectionMarkers = this.languageMode.injectionsMarkerLayer
       .findMarkers({intersectsRange: range})
       .filter(marker => marker.parentLanguageLayer === this)
 
@@ -680,7 +700,7 @@ class LanguageLayer {
         const languageName = injectionPoint.language(node)
         if (!languageName) continue
 
-        const grammar = grammarForLanguageString(languageName)
+        const grammar = this.languageMode.grammarForLanguageString(languageName)
         if (!grammar) continue
 
         const contentNodes = injectionPoint.content(node)
@@ -695,7 +715,7 @@ class LanguageLayer {
           m.languageLayer.grammar === grammar
         )
         if (!marker) {
-          marker = injectionsMarkerLayer.markRange(injectionRange)
+          marker = this.languageMode.injectionsMarkerLayer.markRange(injectionRange)
           marker.languageLayer = new LanguageLayer(this.languageMode, grammar, injectionPoint.contentChildTypes)
           marker.parentLanguageLayer = this
         }