Ensure graph queries with ids that have slashes work

Do this by actually walking the interpretation directory.

Move the directory walker from tests to prod and make it async. Also
add tests for it.

And add a warning on graph views to let users know that it is not
production quality.

Finally, change the interpreted directory to be `graphResults` instead
of `interpretedResults.sarif`.

extensions/ql-vscode/src/cli.ts

@@ -13,7 +13,7 @@ import { CancellationToken, Disposable, Uri } from 'vscode';
 import { BQRSInfo, DecodedBqrsChunk } from './pure/bqrs-cli-types';
 import { CliConfig } from './config';
 import { DistributionProvider, FindDistributionResultKind } from './distribution';
-import { assertNever } from './pure/helpers-pure';
+import { assertNever, walkDirectory } from './pure/helpers-pure';
 import { QueryMetadata, SortDirection } from './pure/interface-types';
 import { Logger, ProgressReporter } from './logging';
 import { CompilationMessage } from './pure/messages';
@@ -729,11 +729,15 @@ export class CodeQLCliServer implements Disposable {
     return await sarifParser(interpretedResultsPath);
   }
 
+  // Warning: this function is untenable for large dot files,
   async readDotFiles(dir: string): Promise<string[]> {
-    return Promise.all((await fs.readdir(dir))
-      .filter(name => path.extname(name).toLowerCase() === '.dot')
-      .map(file => fs.readFile(path.join(dir, file), 'utf8'))
-    );
+    const dotFiles: Promise<string>[] = [];
+    for await (const file of walkDirectory(dir)) {
+      if (file.endsWith('.dot')) {
+        dotFiles.push(fs.readFile(file, 'utf8'));
+      }
+    }
+    return Promise.all(dotFiles);
   }
 
   async interpretBqrsGraph(metadata: QueryMetadata, resultsPath: string, interpretedResultsPath: string, sourceInfo?: SourceInfo): Promise<string[]> {
@@ -1246,7 +1250,7 @@ export class CliVersionConstraint {
 
   /**
    * CLI version where the `--evaluator-log` and related options to the query server were introduced,
-   * on a per-query server basis. 
+   * on a per-query server basis.
    */
   public static CLI_VERSION_WITH_STRUCTURED_EVAL_LOG = new SemVer('2.8.2');
 

extensions/ql-vscode/src/helpers.ts

@@ -558,3 +558,28 @@ export async function createTimestampFile(storagePath: string) {
   await fs.ensureDir(storagePath);
   await fs.writeFile(timestampPath, Date.now().toString(), 'utf8');
 }
+
+
+/**
+ * Recursively walk a directory and return the full path to all files found.
+ * Symbolic links are ignored.
+ *
+ * @param dir the directory to walk
+ *
+ * @return An iterator of the full path to all files recursively found in the directory.
+ */
+ export async function* walkDirectory(dir: string): AsyncIterableIterator<string> {
+  const seenFiles = new Set<string>();
+  for await (const d of await fs.opendir(dir)) {
+    const entry = path.join(dir, d.name);
+    if (seenFiles.has(entry)) {
+      continue;
+    }
+    seenFiles.add(entry);
+    if (d.isDirectory()) {
+      yield* walkDirectory(entry);
+    } else if (d.isFile()) {
+      yield entry;
+    }
+  }
+}

extensions/ql-vscode/src/pure/helpers-pure.ts

@@ -1,3 +1,4 @@
+
 /**
  * helpers-pure.ts
  * ------------

extensions/ql-vscode/src/run-queries.ts

@@ -86,7 +86,11 @@ export class QueryEvaluationInfo {
   get resultsPaths() {
     return {
       resultsPath: path.join(this.querySaveDir, 'results.bqrs'),
-      interpretedResultsPath: path.join(this.querySaveDir, 'interpretedResults.sarif'),
+      interpretedResultsPath: path.join(this.querySaveDir,
+        this.metadata?.kind === 'graph'
+          ? 'graphResults'
+          : 'interpretedResults.sarif'
+      ),
     };
   }
 

extensions/ql-vscode/src/view/graph.tsx

@@ -25,6 +25,9 @@ export class Graph extends React.Component<GraphProps> {
     }
 
     return <>
+      <div className={graphClassName}>
+        <strong>Warning:</strong> The Graph Viewer is not a publicly released feature and will crash on large graphs.
+      </div>
       <div id={graphId} className={graphClassName}><span>Rendering graph...</span></div>
     </>;
   };

extensions/ql-vscode/src/vscode-tests/directory-walker.ts

@@ -1,23 +0,0 @@
-import * as path from 'path';
-import * as fs from 'fs-extra';
-
-/**
- * Recursively walk a directory and return the full path to all files found.
- * Note that this function uses synchronous fs calls, so it should only be used in tests.
- *
- * @param dir the directory to walk
- *
- * @return An iterator of the full path to all files recursively found in the directory.
- */
-export function* walk(dir: string): IterableIterator<string> {
-  const files = fs.readdirSync(dir);
-  for (const file of files) {
-    const filePath = path.join(dir, file);
-    const stat = fs.statSync(filePath);
-    if (stat.isDirectory()) {
-      yield* walk(filePath);
-    } else {
-      yield filePath;
-    }
-  }
-}

extensions/ql-vscode/src/vscode-tests/no-workspace/helpers.test.ts

@@ -1,11 +1,23 @@
 import { expect } from 'chai';
 import 'mocha';
-import { EnvironmentVariableCollection, EnvironmentVariableMutator, Event, ExtensionContext, ExtensionMode, Memento, SecretStorage, SecretStorageChangeEvent, Uri, window } from 'vscode';
+import {
+  EnvironmentVariableCollection,
+  EnvironmentVariableMutator,
+  Event,
+  ExtensionContext,
+  ExtensionMode,
+  Memento,
+  SecretStorage,
+  SecretStorageChangeEvent,
+  Uri,
+  window
+} from 'vscode';
 import * as yaml from 'js-yaml';
 import * as tmp from 'tmp';
 import * as path from 'path';
 import * as fs from 'fs-extra';
 import * as sinon from 'sinon';
+import { DirResult } from 'tmp';
 
 import {
   getInitialQueryContents,
@@ -13,7 +25,8 @@ import {
   isLikelyDbLanguageFolder,
   showBinaryChoiceDialog,
   showBinaryChoiceWithUrlDialog,
-  showInformationMessageWithAction
+  showInformationMessageWithAction,
+  walkDirectory
 } from '../../helpers';
 import { reportStreamProgress } from '../../commandRunner';
 import Sinon = require('sinon');
@@ -377,3 +390,65 @@ describe('helpers', () => {
     });
   });
 });
+
+describe('walkDirectory', () => {
+  let tmpDir: DirResult;
+  let dir: string;
+  let dir2: string;
+
+  beforeEach(() => {
+    tmpDir = tmp.dirSync({ unsafeCleanup: true });
+    dir = path.join(tmpDir.name, 'dir');
+    fs.ensureDirSync(dir);
+    dir2 = path.join(tmpDir.name, 'dir2');
+  });
+
+  afterEach(() => {
+    tmpDir.removeCallback();
+  });
+
+  it('should walk a directory', async () => {
+    const file1 = path.join(dir, 'file1');
+    const file2 = path.join(dir, 'file2');
+    const file3 = path.join(dir, 'file3');
+    const dir3 = path.join(dir, 'dir3');
+    const file4 = path.join(dir, 'file4');
+    const file5 = path.join(dir, 'file5');
+    const file6 = path.join(dir, 'file6');
+
+    // These symlinks link back to paths that are already existing, so ignore.
+    const symLinkFile7 = path.join(dir, 'symlink0');
+    const symlinkDir = path.join(dir2, 'symlink1');
+
+    // some symlinks that point outside of the base dir.
+    const file8 = path.join(tmpDir.name, 'file8');
+    const file9 = path.join(dir2, 'file8');
+    const symlinkDir2 = path.join(dir2, 'symlink2');
+    const symlinkFile2 = path.join(dir2, 'symlinkFile3');
+
+    fs.ensureDirSync(dir2);
+    fs.ensureDirSync(dir3);
+
+    fs.writeFileSync(file1, 'file1');
+    fs.writeFileSync(file2, 'file2');
+    fs.writeFileSync(file3, 'file3');
+    fs.writeFileSync(file4, 'file4');
+    fs.writeFileSync(file5, 'file5');
+    fs.writeFileSync(file6, 'file6');
+    fs.writeFileSync(file8, 'file8');
+    fs.writeFileSync(file9, 'file9');
+
+    fs.symlinkSync(file6, symLinkFile7, 'file');
+    fs.symlinkSync(dir3, symlinkDir, 'dir');
+    fs.symlinkSync(file8, symlinkFile2, 'file');
+    fs.symlinkSync(dir2, symlinkDir2, 'dir');
+
+    const files = [];
+    for await (const file of walkDirectory(dir)) {
+      files.push(file);
+    }
+
+    // Only real files should be returned.
+    expect(files.sort()).to.deep.eq([file1, file2, file3, file4, file5, file6]);
+  });
+});

extensions/ql-vscode/src/vscode-tests/no-workspace/remote-query-history.test.ts

@@ -17,7 +17,7 @@ import { AnalysesResultsManager } from '../../remote-queries/analyses-results-ma
 import { RemoteQueryResult } from '../../remote-queries/shared/remote-query-result';
 import { DisposableBucket } from '../disposable-bucket';
 import { testDisposeHandler } from '../test-dispose-handler';
-import { walk } from '../directory-walker';
+import { walkDirectory } from '../../pure/helpers-pure';
 
 chai.use(chaiAsPromised);
 const expect = chai.expect;
@@ -41,10 +41,10 @@ describe('Remote queries and query history manager', function() {
   let showTextDocumentSpy: sinon.SinonSpy;
   let openTextDocumentSpy: sinon.SinonSpy;
 
-  beforeEach(() => {
+  beforeEach(async () => {
     // Since these tests change the state of the query history manager, we need to copy the original
     // to a temporary folder where we can manipulate it for tests
-    copyHistoryState();
+    await copyHistoryState();
   });
 
   afterEach(() => {
@@ -321,12 +321,12 @@ describe('Remote queries and query history manager', function() {
     });
   });
 
-  function copyHistoryState() {
+  async function copyHistoryState() {
     fs.ensureDirSync(STORAGE_DIR);
     fs.copySync(path.join(__dirname, 'data/remote-queries/'), path.join(tmpDir.name, 'remote-queries'));
 
     // also, replace the files with "PLACEHOLDER" so that they have the correct directory
-    for (const p of walk(STORAGE_DIR)) {
+    for await (const p of walkDirectory(STORAGE_DIR)) {
       replacePlaceholder(path.join(p));
     }
   }

extensions/ql-vscode/test/pure-tests/helpers-pure.test.ts

@@ -1,5 +1,6 @@
 import { fail } from 'assert';
 import { expect } from 'chai';
+
 import { asyncFilter } from '../../src/pure/helpers-pure';
 
 describe('helpers-pure', () => {