Implement jumping to modules imported by relative imports, and:

- fix and old regression which rejected results of the first filtering step of definitions
- fix the element counter mixing up variables and properties instead of focusing on one type only
- add more Python jump tests, reuse testing code
- add test for CodeMirrorExtension.selectToken(), using a challenging edge case

Author: krassowski

package.json

@@ -38,6 +38,7 @@
     "@jupyterlab/codemirror": "^1.0.0",
     "@jupyterlab/coreutils": "^3.0.0",
     "@jupyterlab/docmanager": "^1.0.0",
+    "@jupyterlab/docregistry": "^1.0.0",
     "@jupyterlab/fileeditor": "^1.0.0",
     "@jupyterlab/notebook": "^1.0.0",
     "@jupyterlab/services": "^4.0.0",

src/editors/codemirror/extension.spec.ts

@@ -0,0 +1,76 @@
+import { expect } from 'chai';
+
+import { CodeEditor } from "@jupyterlab/codeeditor";
+import { CodeMirrorEditor, Mode } from '@jupyterlab/codemirror';
+
+import { CodeMirrorTokensProvider } from "../../editors/codemirror/tokens";
+
+import { Jumper, matchToken } from "../../testutils";
+import { CodeMirrorExtension } from "../../editors/codemirror";
+
+
+describe('CodeMirrorExtension', () => {
+
+  let editor: CodeMirrorEditor;
+  let tokensProvider: CodeMirrorTokensProvider;
+  let model: CodeEditor.Model;
+  let host: HTMLElement;
+  let jumper: Jumper;
+  let extension: CodeMirrorExtension;
+
+  beforeEach(() => {
+    host = document.createElement('div');
+    document.body.appendChild(host);
+
+    model = new CodeEditor.Model({mimeType: 'text/x-python'});
+    editor = new CodeMirrorEditor({host, model, config: {mode: 'python'}});
+    tokensProvider = new CodeMirrorTokensProvider(editor);
+
+    jumper = new Jumper(editor);
+
+    extension = new CodeMirrorExtension(editor, jumper);
+    extension.connect();
+  });
+
+  afterEach(() => {
+    editor.dispose();
+    document.body.removeChild(host);
+  });
+
+  describe('#selectToken', () => {
+
+    it('should select token based on given DOM element of event origin', () => {
+      model.value.text = `def x():
+    a = 1
+    a
+
+a = 1
+y = a`;
+
+      let tokens = tokensProvider.getTokens();
+      let token = matchToken(tokens, 'a', 4);
+
+      let position = {line: 5, column: 5};
+
+      editor.setCursorPosition(position);
+
+      expect(editor.getCursorPosition()).to.deep.equal(position);
+      expect(editor.getTokenForPosition(position)).to.deep.equal(token);
+
+      let mode = editor.editor.getOption('mode');
+      Mode.run(model.value.text, mode, editor.host);
+
+      let node = editor.host;
+
+      while (node.className !== 'cm-variable')
+        node = node.lastElementChild as HTMLElement;
+
+      // sanity checks
+      expect(node.innerHTML).to.be.equal('a');
+      expect(node.previousElementSibling.previousElementSibling.innerHTML).to.be.equal('y');
+
+      expect(extension.selectToken('a', node)).to.deep.equal(token);
+    });
+
+  });
+});

src/editors/codemirror/extension.ts

@@ -106,14 +106,28 @@ export class CodeMirrorExtension extends CodeMirrorTokensProvider implements IEd
 
     let cellTokens = this.editor.getTokens();
 
-    let usagesBeforeTarget = CodeMirrorExtension._countUsagesBefore(lookupName, target);
+    let typeFilterOn = (
+      target.className.includes('cm-variable')
+      ||
+      target.className.includes('cm-property')
+    );
+
+    let lookupType = (
+      target.className.indexOf('cm-variable') !== -1
+        ? 'variable'
+        : 'property'
+    );
+
+    let classFilter = 'cm-' + lookupType;
+
+    let usagesBeforeTarget = CodeMirrorExtension._countUsagesBefore(lookupName, target, classFilter, typeFilterOn);
 
     // select relevant token
     let token = null;
     let matchedTokensCount = 0;
     for (let j = 0; j < cellTokens.length; j++) {
       let testedToken = cellTokens[j];
-      if (testedToken.value === lookupName) {
+      if (testedToken.value === lookupName && (!typeFilterOn || lookupType === testedToken.type)) {
         matchedTokensCount += 1;
         if (matchedTokensCount - 1 === usagesBeforeTarget) {
           token = testedToken;
@@ -131,48 +145,63 @@ export class CodeMirrorExtension extends CodeMirrorTokensProvider implements IEd
       token = {
         value: lookupName,
         offset: 0, // dummy offset
-        type:
-          target.className.indexOf('cm-variable') !== -1
-            ? 'variable'
-            : 'property'
+        type: lookupType
       };
     }
 
     return token;
   }
 
-  static _countUsagesBefore(lookupName: string, target: Node) {
+  static _countUsagesBefore(lookupName: string, target: Node, classFilter: string, classFilterOn: boolean) {
 
     // count tokens with same value that occur before
     // (not all the tokens - to reduce the hurdle of
     // mapping DOM into tokens)
     let usagesBeforeTarget = -1;
     let sibling = target as Node;
 
-    while (sibling != null) {
-      if (sibling.textContent === lookupName) {
+    const root = sibling.getRootNode();
+
+    // usually should not exceed that level, but to prevent huge files from trashing the UI...
+    let max_iter = 10000;
+
+    function stop_condition(node: Node) {
+      return !node || (node.nodeType == 1 && (node as HTMLElement).className.includes('CodeMirror-lines'))
+    }
+
+    while (!stop_condition(sibling) && !sibling.isEqualNode(root) && max_iter) {
+
+      if (
+        sibling.textContent === lookupName &&
+        (
+          !classFilterOn ||
+          (
+            sibling.nodeType === 1 &&
+            (sibling as HTMLElement).className.includes(classFilter)
+          )
+        )
+      ) {
         usagesBeforeTarget += 1;
       }
 
       let nextSibling = sibling.previousSibling;
 
-      if (nextSibling == null) {
-        // Try to traverse to previous line (if there is one).
-
-        // The additional parent (child) which is traversed
-        // both ways is a non-relevant presentation container.
-        let thisLine = sibling.parentNode.parentNode as HTMLElement;
-        let previousLine = thisLine.previousElementSibling;
-
-        // is is really a line?
-        if (
-          previousLine &&
-          previousLine.className.indexOf('CodeMirror-line') !== -1
-        ) {
-          nextSibling = previousLine.firstChild.lastChild;
+      while (nextSibling == null) {
+        while(!sibling.previousSibling) {
+          sibling = sibling.parentNode;
+          if (stop_condition(sibling)) {
+            return usagesBeforeTarget;
+          }
+        }
+        sibling = sibling.previousSibling;
+        while (sibling.lastChild && sibling.textContent != lookupName) {
+          sibling = sibling.lastChild;
         }
+        nextSibling = sibling;
       }
       sibling = nextSibling;
+
+      max_iter -= 1;
     }
 
     return usagesBeforeTarget;

src/index.ts

@@ -39,7 +39,7 @@ const plugin: JupyterFrontEndPlugin<void> = {
 
       if (fileEditor.editor instanceof CodeMirrorEditor) {
 
-        let jumper = new FileEditorJumper(fileEditor, jump_history, documentManager);
+        let jumper = new FileEditorJumper(widget, jump_history, documentManager);
         let extension = new CodeMirrorExtension(fileEditor.editor, jumper);
 
         extension.connect();
@@ -159,9 +159,10 @@ const plugin: JupyterFrontEndPlugin<void> = {
     app.commands.addCommand(cmdIds.jumpFileEditor, {
       label: 'Jump to definition',
       execute: () => {
-        let fileEditor = fileEditorTracker.currentWidget.content;
+        let fileEditorWidget = fileEditorTracker.currentWidget;
+        let fileEditor = fileEditorWidget.content;
 
-        let jumper = new FileEditorJumper(fileEditor, jump_history, documentManager);
+        let jumper = new FileEditorJumper(fileEditorWidget, jump_history, documentManager);
         let editor = fileEditor.editor;
 
         let position = editor.getCursorPosition();
@@ -175,9 +176,9 @@ const plugin: JupyterFrontEndPlugin<void> = {
     app.commands.addCommand(cmdIds.jumpBackFileEditor, {
       label: 'Jump back',
       execute: () => {
-        let fileEditor = fileEditorTracker.currentWidget.content;
+        let fileEditorWidget = fileEditorTracker.currentWidget;
 
-        let jumper = new FileEditorJumper(fileEditor, jump_history, documentManager);
+        let jumper = new FileEditorJumper(fileEditorWidget, jump_history, documentManager);
         jumper.jump_back()
       },
       isEnabled: isEnabled(fileEditorTracker)

src/jumpers/fileeditor.ts

@@ -4,26 +4,33 @@ import { CodeJumper } from "./jumper";
 import { JumpHistory } from "../history";
 import { TokenContext } from "../languages/analyzer";
 import { IDocumentManager } from "@jupyterlab/docmanager";
+import { IDocumentWidget } from "@jupyterlab/docregistry";
 
 
 export class FileEditorJumper extends CodeJumper {
 
   editor: FileEditor;
   language: string;
   history: JumpHistory;
+  widget: IDocumentWidget;
 
-  constructor(editor: FileEditor, history: JumpHistory, document_manager: IDocumentManager) {
+  constructor(editor_widget: IDocumentWidget<FileEditor>, history: JumpHistory, document_manager: IDocumentManager) {
     super();
+    this.widget = editor_widget;
     this.document_manager = document_manager;
-    this.editor = editor;
+    this.editor = editor_widget.content;
     this.history = history;
-    this.setLanguageFromMime(editor.model.mimeType);
+    this.setLanguageFromMime(this.editor.model.mimeType);
 
-    editor.model.mimeTypeChanged.connect((session, mimeChanged) => {
+    this.editor.model.mimeTypeChanged.connect((session, mimeChanged) => {
       this.setLanguageFromMime(mimeChanged.newValue)
     });
   }
 
+  get cwd() {
+    return this.widget.context.path.split('/').slice(0, -1).join('/')
+  }
+
   setLanguageFromMime(mime: string){
     let type = mime.replace('text/x-', '');
     switch (type) {

src/jumpers/jumper.ts

@@ -101,12 +101,14 @@ export abstract class CodeJumper {
       // try to find variable assignment
       let definitions = analyzer.getDefinitions(token.value);
 
-
       if (definitions.length) {
-        // Get the last definition / assignment that appears before
-        // token of origin (is in an earlier cell or has lower offset),
-        let filtered = definitions.filter(
-          otherToken => i < stopIndex || otherToken.offset < originToken.offset
+        // Get definitions / assignments that appear before
+        // token of origin (are in an earlier cells or have lower offset),
+        let in_earlier_cell = i < stopIndex;
+        let filtered = (
+          in_earlier_cell
+            ? definitions  // all are in an earlier cell
+            : definitions.filter( otherToken => otherToken.offset < originToken.offset) // all are in same cell
         );
 
         // but ignore ones that are part of the same assignment expression,
@@ -115,11 +117,12 @@ export abstract class CodeJumper {
         // >>> a = a + 1
         // clicking on the last 'a' should jump to the first line,
         // and not to beginning of the second line.
-        filtered = definitions.filter(otherToken => {
+        filtered = filtered.filter(otherToken => {
           // If otherToken is in previous cell, we don't need to worry.
           if (i < stopIndex) {
             return true;
           }
+
           return !analyzer.isTokenInSameAssignmentExpression(
             otherToken, token
           );
@@ -169,13 +172,21 @@ export abstract class CodeJumper {
     return null
   }
 
+  abstract get cwd(): string;
+
   protected jump_to_cross_file_reference(context: TokenContext, cell_of_origin_analyzer: LanguageAnalyzer) {
 
     let potential_paths = cell_of_origin_analyzer.guessReferencePath(context);
+    if(this.cwd) {
+      let prefixed_with_cwd = potential_paths.map(path => this.cwd + '/' + path);
+      potential_paths = prefixed_with_cwd.concat(potential_paths);
+    }
+
+    let code = cell_of_origin_analyzer.referencePathQuery(context);
 
-    if (cell_of_origin_analyzer.supportsKernel && this.kernel) {
+    if (cell_of_origin_analyzer.supportsKernel && this.kernel && code) {
       cell_of_origin_analyzer.requestReferencePathFromKernel(
-        context, this.kernel,
+        code, this.kernel,
         msg => this.handle_path_from_kernel(msg, potential_paths)
       );
     } else {

src/jumpers/notebook.ts

@@ -28,6 +28,10 @@ export class NotebookJumper extends CodeJumper {
     return this.widget.session.kernel;
   }
 
+  get cwd() {
+    return this.widget.model.modelDB.basePath.split('/').slice(0, -1).join('/')
+  }
+
   get editors() {
     return this.notebook.widgets.map((cell) => cell.editor)
   }

src/languages/analyzer.ts

@@ -85,11 +85,7 @@ export abstract class LanguageAnalyzer {
     return '';
   }
 
-  requestReferencePathFromKernel(context: TokenContext, kernel: Kernel.IKernelConnection, callback: (msg: KernelMessage.IIOPubMessage) => any) {
-    let code = this.referencePathQuery(context);
-
-    if(!code)
-      return;
+  requestReferencePathFromKernel(code: string, kernel: Kernel.IKernelConnection, callback: (msg: KernelMessage.IIOPubMessage) => any) {
 
     let request = {code: code, stop_on_error: false, silent: true};
     kernel.ready.then(() => {
@@ -109,7 +105,7 @@ export abstract class LanguageAnalyzer {
 
   _get_token_index(token: CodeEditor.IToken) {
     this._maybe_setup_tokens();
-    return this.tokens.findIndex(t => (t.value == token.value && t.offset == token.offset))
+    return this.tokens.findIndex(t => (t.value == token.value && t.offset == token.offset && t.type == token.type))
   }
 
   isDefinition(token: CodeEditor.IToken, i: number) {