Merge pull request #1565 from github/aeisenberg/db-folder-check

Use better heuristics when checking db folders

extensions/ql-vscode/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## [UNRELEASED]
 
+- Fix a bug where it was not possible to add a database folder if the folder name starts with `db-`. [#1565](https://github.com/github/vscode-codeql/pull/1565)
+
 ## 1.7.0 - 20 September 2022
 
 - Remove ability to download databases from LGTM. [#1467](https://github.com/github/vscode-codeql/pull/1467)

extensions/ql-vscode/src/databases-ui.ts

@@ -750,7 +750,7 @@ export class DatabaseUI extends DisposableObject {
    * Perform some heuristics to ensure a proper database location is chosen.
    *
    * 1. If the selected URI to add is a file, choose the containing directory
-   * 2. If the selected URI is a directory matching db-*, choose the containing directory
+   * 2. If the selected URI appears to be a db language folder, choose the containing directory
    * 3. choose the current directory
    *
    * @param uri a URI that is a database folder or inside it
@@ -763,7 +763,7 @@ export class DatabaseUI extends DisposableObject {
       dbPath = path.dirname(dbPath);
     }
 
-    if (isLikelyDbLanguageFolder(dbPath)) {
+    if (await isLikelyDbLanguageFolder(dbPath)) {
       dbPath = path.dirname(dbPath);
     }
     return Uri.file(dbPath);

extensions/ql-vscode/src/helpers.ts

@@ -470,9 +470,9 @@ export function getInitialQueryContents(language: string, dbscheme: string) {
 
 /**
  * Heuristically determines if the directory passed in corresponds
- * to a database root.
- *
- * @param maybeRoot
+ * to a database root. A database root is a directory that contains
+ * a codeql-database.yml or (historically) a .dbinfo file. It also
+ * contains a folder starting with `db-`.
  */
 export async function isLikelyDatabaseRoot(maybeRoot: string) {
   const [a, b, c] = (await Promise.all([
@@ -484,11 +484,14 @@ export async function isLikelyDatabaseRoot(maybeRoot: string) {
     glob('db-*/', { cwd: maybeRoot })
   ]));
 
-  return !!((a || b) && c);
+  return ((a || b) && c.length > 0);
 }
 
-export function isLikelyDbLanguageFolder(dbPath: string) {
-  return !!path.basename(dbPath).startsWith('db-');
+/**
+ * A language folder is any folder starting with `db-` that is itself not a database root.
+ */
+export async function isLikelyDbLanguageFolder(dbPath: string) {
+  return path.basename(dbPath).startsWith('db-') && !(await isLikelyDatabaseRoot(dbPath));
 }
 
 /**

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

@@ -21,6 +21,7 @@ import { DirResult } from 'tmp';
 import {
   getInitialQueryContents,
   InvocationRateLimiter,
+  isLikelyDatabaseRoot,
   isLikelyDbLanguageFolder,
   showBinaryChoiceDialog,
   showBinaryChoiceWithUrlDialog,
@@ -150,11 +151,65 @@ describe('helpers', () => {
     it('should get initial query contents when nothing is known', () => {
       expect(getInitialQueryContents('', 'hucairz')).to.eq('select ""');
     });
+
   });
 
-  it('should find likely db language folders', () => {
-    expect(isLikelyDbLanguageFolder('db-javascript')).to.be.true;
-    expect(isLikelyDbLanguageFolder('dbnot-a-db')).to.be.false;
+  describe('likely database tests', () => {
+    let dir: tmp.DirResult;
+    beforeEach(() => {
+      dir = tmp.dirSync();
+    });
+
+    afterEach(() => {
+      dir.removeCallback();
+    });
+
+    it('should likely be a database root: codeql-database.yml', async () => {
+      const dbFolder = path.join(dir.name, 'db');
+      fs.mkdirSync(dbFolder);
+      fs.mkdirSync(path.join(dbFolder, 'db-python'));
+      fs.writeFileSync(path.join(dbFolder, 'codeql-database.yml'), '', 'utf8');
+
+      expect(await isLikelyDatabaseRoot(dbFolder)).to.be.true;
+    });
+
+    it('should likely be a database root: .dbinfo', async () => {
+      const dbFolder = path.join(dir.name, 'db');
+      fs.mkdirSync(dbFolder);
+      fs.mkdirSync(path.join(dbFolder, 'db-python'));
+      fs.writeFileSync(path.join(dbFolder, '.dbinfo'), '', 'utf8');
+
+      expect(await isLikelyDatabaseRoot(dbFolder)).to.be.true;
+    });
+
+    it('should likely NOT be a database root: empty dir', async () => {
+      const dbFolder = path.join(dir.name, 'db');
+      fs.mkdirSync(dbFolder);
+      fs.mkdirSync(path.join(dbFolder, 'db-python'));
+
+      expect(await isLikelyDatabaseRoot(dbFolder)).to.be.false;
+    });
+
+    it('should likely NOT be a database root: no db language folder', async () => {
+      const dbFolder = path.join(dir.name, 'db');
+      fs.mkdirSync(dbFolder);
+      fs.writeFileSync(path.join(dbFolder, '.dbinfo'), '', 'utf8');
+
+      expect(await isLikelyDatabaseRoot(dbFolder)).to.be.false;
+    });
+
+    it('should find likely db language folder', async () => {
+      const dbFolder = path.join(dir.name, 'db-python');
+      fs.mkdirSync(dbFolder);
+      fs.mkdirSync(path.join(dbFolder, 'db-python'));
+      fs.writeFileSync(path.join(dbFolder, 'codeql-database.yml'), '', 'utf8');
+
+      // not a db folder since there is a db-python folder inside this one
+      expect(await isLikelyDbLanguageFolder(dbFolder)).to.be.false;
+
+      const nestedDbPythonFolder = path.join(dbFolder, 'db-python');
+      expect(await isLikelyDbLanguageFolder(nestedDbPythonFolder)).to.be.true;
+    });
   });
 
   class MockExtensionContext implements ExtensionContext {