Fix invalid file comparison for changing ast viewer location

This fixes a bug where the ast viewer was not updating its source
location when a user clicks on different parts of a file.

The problem was that the file name of the AST viewer was being stored as
a base name, which was getting compared with the full URI string of the
current file.

This fixes the comparison to ensure that the full URI strings are always
being compared.
This commit is contained in:
Andrew Eisenberg
2022-03-24 12:36:17 -07:00
parent 3d21b203be
commit 0fa91f32cb
5 changed files with 27 additions and 19 deletions

View File

@@ -10,7 +10,8 @@ import {
TextEditorSelectionChangeEvent,
TextEditorSelectionChangeKind,
Location,
Range
Range,
Uri
} from 'vscode';
import * as path from 'path';
@@ -104,7 +105,7 @@ class AstViewerDataProvider extends DisposableObject implements TreeDataProvider
export class AstViewer extends DisposableObject {
private treeView: TreeView<AstItem>;
private treeDataProvider: AstViewerDataProvider;
private currentFile: string | undefined;
private currentFileUri: Uri | undefined;
constructor() {
super();
@@ -125,12 +126,12 @@ export class AstViewer extends DisposableObject {
this.push(window.onDidChangeTextEditorSelection(this.updateTreeSelection, this));
}
updateRoots(roots: AstItem[], db: DatabaseItem, fileName: string) {
updateRoots(roots: AstItem[], db: DatabaseItem, fileUri: Uri) {
this.treeDataProvider.roots = roots;
this.treeDataProvider.db = db;
this.treeDataProvider.refresh();
this.treeView.message = `AST for ${path.basename(fileName)}`;
this.currentFile = fileName;
this.treeView.message = `AST for ${path.basename(fileUri.fsPath)}`;
this.currentFileUri = fileUri;
// Handle error on reveal. This could happen if
// the tree view is disposed during the reveal.
this.treeView.reveal(roots[0], { focus: false })?.then(
@@ -174,7 +175,7 @@ export class AstViewer extends DisposableObject {
if (
this.treeView.visible &&
e.textEditor.document.uri.fsPath === this.currentFile &&
e.textEditor.document.uri.fsPath === this.currentFileUri?.fsPath &&
e.selections.length === 1
) {
const selection = e.selections[0];
@@ -199,6 +200,6 @@ export class AstViewer extends DisposableObject {
this.treeDataProvider.db = undefined;
this.treeDataProvider.refresh();
this.treeView.message = undefined;
this.currentFile = undefined;
this.currentFileUri = undefined;
}
}

View File

@@ -4,6 +4,7 @@ import { DecodedBqrsChunk, BqrsId, EntityValue } from '../pure/bqrs-cli-types';
import { DatabaseItem } from '../databases';
import { ChildAstItem, AstItem } from '../astViewer';
import fileRangeFromURI from './fileRangeFromURI';
import { Uri } from 'vscode';
/**
* A class that wraps a tree of QL results from a query that
@@ -17,7 +18,7 @@ export default class AstBuilder {
queryResults: QueryWithResults,
private cli: CodeQLCliServer,
public db: DatabaseItem,
public fileName: string
public fileName: Uri
) {
this.bqrsPath = queryResults.query.resultsPaths.resultsPath;
}

View File

@@ -10,7 +10,6 @@ import {
TextDocument,
Uri
} from 'vscode';
import * as path from 'path';
import { decodeSourceArchiveUri, encodeArchiveBasePath, zipArchiveScheme } from '../archive-filesystem-provider';
import { CodeQLCliServer } from '../cli';
@@ -160,7 +159,7 @@ export class TemplatePrintAstProvider {
return new AstBuilder(
query, this.cli,
this.dbm.findDatabaseItem(dbUri)!,
path.basename(fileUri.fsPath),
fileUri,
);
}

View File

@@ -5,7 +5,7 @@ import * as sinon from 'sinon';
import * as yaml from 'js-yaml';
import { AstViewer, AstItem } from '../../astViewer';
import { commands, Range } from 'vscode';
import { commands, Range, Uri } from 'vscode';
import { DatabaseItem } from '../../databases';
import { testDisposeHandler } from '../test-dispose-handler';
@@ -40,7 +40,7 @@ describe('AstViewer', () => {
it('should update the viewer roots', () => {
const item = {} as DatabaseItem;
viewer = new AstViewer();
viewer.updateRoots(astRoots, item, 'def/abc');
viewer.updateRoots(astRoots, item, Uri.file('def/abc'));
expect((viewer as any).treeDataProvider.roots).to.eq(astRoots);
expect((viewer as any).treeDataProvider.db).to.eq(item);
@@ -59,25 +59,31 @@ describe('AstViewer', () => {
doSelectionTest(expr, expr.fileLocation?.range);
});
it('should select nothing', () => {
it('should select nothing because of no overlap in range', () => {
doSelectionTest(undefined, new Range(2, 3, 4, 5));
});
it('should select nothing because of different file', () => {
doSelectionTest(undefined, astRoots[0].fileLocation?.range, Uri.file('def'));
});
const defaultUri = Uri.file('def/abc');
function doSelectionTest(
expectedSelection: any,
selectionRange: Range | undefined,
fsPath = 'def/abc',
fileUri = defaultUri
) {
const item = {} as DatabaseItem;
viewer = new AstViewer();
viewer.updateRoots(astRoots, item, fsPath);
viewer.updateRoots(astRoots, item, defaultUri);
const spy = sandbox.spy();
(viewer as any).treeView.reveal = spy;
Object.defineProperty((viewer as any).treeView, 'visible', {
value: true
});
const mockEvent = createMockEvent(selectionRange, fsPath);
const mockEvent = createMockEvent(selectionRange, fileUri);
(viewer as any).updateTreeSelection(mockEvent);
if (expectedSelection) {
expect(spy).to.have.been.calledWith(expectedSelection);
@@ -88,7 +94,7 @@ describe('AstViewer', () => {
function createMockEvent(
selectionRange: Range | undefined,
fsPath: string,
uri: Uri,
) {
return {
selections: [{
@@ -98,7 +104,7 @@ describe('AstViewer', () => {
textEditor: {
document: {
uri: {
fsPath
fsPath: uri.fsPath
}
}
}

View File

@@ -7,6 +7,7 @@ import AstBuilder from '../../../contextual/astBuilder';
import { QueryWithResults } from '../../../run-queries';
import { CodeQLCliServer } from '../../../cli';
import { DatabaseItem } from '../../../databases';
import { Uri } from 'vscode';
chai.use(chaiAsPromised);
const expect = chai.expect;
@@ -145,7 +146,7 @@ describe('AstBuilder', () => {
resultsPath: '/a/b/c'
}
}
} as QueryWithResults, mockCli, {} as DatabaseItem, '');
} as QueryWithResults, mockCli, {} as DatabaseItem, Uri.file(''));
}
function mockDecode(resultSet: 'nodes' | 'edges' | 'graphProperties') {