Date CHANGELOG for release (#579)

* node-fetch
* bl

.github/workflows/main.yml

@@ -37,7 +37,7 @@ jobs:
           cp dist/*.vsix artifacts
 
       - name: Upload artifacts
-        uses: actions/upload-artifact@master
+        uses: actions/upload-artifact@v2
         if: matrix.os == 'ubuntu-latest'
         with:
           name: vscode-codeql-extension

.github/workflows/release.yml

@@ -63,7 +63,7 @@ jobs:
       # This is just in case the release itself fails and we want to access the built artifacts from Actions.
       # TODO Remove if not useful.
       - name: Upload artifacts
-        uses: actions/upload-artifact@master
+        uses: actions/upload-artifact@v2
         with:
           name: vscode-codeql-extension
           path: artifacts
@@ -114,6 +114,12 @@ jobs:
           NEXT_VERSION="$(npm version patch)"
           echo "::set-output name=next_version::$NEXT_VERSION"
 
+      - name: Add changelog for next release
+        if: success()
+        run: |
+          cd extensions/ql-vscode
+          perl -i -pe 's/^/## \[UNRELEASED\]\n\n/ if($.==3)' CHANGELOG.md
+
       - name: Create version bump PR
         uses: peter-evans/create-pull-request@c7b64af0a489eae91f7890f2c1b63d13cc2d8ab7 # v2.4.2
         if: success()

extensions/ql-vscode/CHANGELOG.md

@@ -1,5 +1,11 @@
 # CodeQL for Visual Studio Code: Changelog
 
+## 1.3.3 - 16 September 2020
+
+- Fix display of raw results entities with label but no url.
+- Fix bug where sort order is forgotten when changing raw results page.
+- Avoid showing a location link in results view when a result item has an empty location.
+
 ## 1.3.2 - 12 August 2020
 
 - Fix error with choosing qlpack search path.

extensions/ql-vscode/package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "vscode-codeql",
-  "version": "1.3.2",
+  "version": "1.3.3",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
@@ -372,6 +372,7 @@
       "version": "7.2.0",
       "resolved": "https://registry.npmjs.org/@types/semver/-/semver-7.2.0.tgz",
       "integrity": "sha512-TbB0A8ACUWZt3Y6bQPstW9QNbhNeebdgLX4T/ZfkrswAfUzRiXrgd9seol+X379Wa589Pu4UEx9Uok0D4RjRCQ==",
+      "dev": true,
       "requires": {
         "@types/node": "*"
       },
@@ -379,7 +380,8 @@
         "@types/node": {
           "version": "14.0.23",
           "resolved": "https://registry.npmjs.org/@types/node/-/node-14.0.23.tgz",
-          "integrity": "sha512-Z4U8yDAl5TFkmYsZdFPdjeMa57NOvnaf1tljHzhouaPEp7LCj2JKkejpI1ODviIAQuW4CcQmxkQ77rnLsOOoKw=="
+          "integrity": "sha512-Z4U8yDAl5TFkmYsZdFPdjeMa57NOvnaf1tljHzhouaPEp7LCj2JKkejpI1ODviIAQuW4CcQmxkQ77rnLsOOoKw==",
+          "dev": true
         }
       }
     },
@@ -1374,9 +1376,9 @@
       }
     },
     "bl": {
-      "version": "4.0.2",
-      "resolved": "https://registry.npmjs.org/bl/-/bl-4.0.2.tgz",
-      "integrity": "sha512-j4OH8f6Qg2bGuWfRiltT2HYGx0e1QcBTrK9KAHNMwMZdQnDZFk0ZSYIpADjYCB3U12nicC5tVJwSIhwOWjb4RQ==",
+      "version": "4.0.3",
+      "resolved": "https://registry.npmjs.org/bl/-/bl-4.0.3.tgz",
+      "integrity": "sha512-fs4G6/Hu4/EE+F75J8DuN/0IpQqNjAdC7aEQv7Qt8MHGUH7Ckv2MwTEEeN9QehD0pfIDkMI1bkHYkKy7xHyKIg==",
       "requires": {
         "buffer": "^5.5.0",
         "inherits": "^2.0.4",
@@ -6204,9 +6206,9 @@
       }
     },
     "node-fetch": {
-      "version": "2.6.0",
-      "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.0.tgz",
-      "integrity": "sha512-8dG4H5ujfvFiqDmVu9fQ5bOHUC15JMjMY/Zumv26oOvvVJjM67KF8koCWIabKQ1GJIa9r2mMZscBq/TbdOcmNA=="
+      "version": "2.6.1",
+      "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.1.tgz",
+      "integrity": "sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw=="
     },
     "node-libs-browser": {
       "version": "2.2.1",

extensions/ql-vscode/package.json

@@ -4,7 +4,7 @@
   "description": "CodeQL for Visual Studio Code",
   "author": "GitHub",
   "private": true,
-  "version": "1.3.2",
+  "version": "1.3.3",
   "publisher": "GitHub",
   "license": "MIT",
   "icon": "media/VS-marketplace-CodeQL-icon.png",
@@ -630,7 +630,7 @@
       },
       {
         "view": "codeQLQueryHistory",
-        "contents": "Run the 'CodeQL: Run Query' command on QL query.\n[Run Query](command:codeQL.runQuery)"
+        "contents": "Run the 'CodeQL: Run Query' command on a QL query.\n[Run Query](command:codeQL.runQuery)"
       }
     ]
   },

extensions/ql-vscode/src/adapt.ts

@@ -73,7 +73,7 @@ export function adaptValue(val: ColumnValue): ResultValue {
   }
 
   if (url === undefined) {
-    return 'none';
+    return val.label || '';
   }
 
   return {

extensions/ql-vscode/src/bqrs-utils.ts

@@ -16,17 +16,24 @@ const FILE_LOCATION_REGEX = /file:\/\/(.+):([0-9]+):([0-9]+):([0-9]+):([0-9]+)/;
 export function tryGetResolvableLocation(
   loc: LocationValue | undefined
 ): ResolvableLocationValue | undefined {
+  let resolvedLoc;
   if (loc === undefined) {
-    return undefined;
+    resolvedLoc = undefined;
   } else if (loc.t === LocationStyle.FivePart && loc.file) {
-    return loc;
+    resolvedLoc = loc;
   } else if (loc.t === LocationStyle.WholeFile && loc.file) {
-    return loc;
+    resolvedLoc = loc;
   } else if (loc.t === LocationStyle.String && loc.loc) {
-    return tryGetLocationFromString(loc);
+    resolvedLoc = tryGetLocationFromString(loc);
   } else {
-    return undefined;
+    resolvedLoc = undefined;
   }
+
+  if (resolvedLoc && isEmptyPath(resolvedLoc.file)) {
+    resolvedLoc = undefined;
+  }
+
+  return resolvedLoc;
 }
 
 export function tryGetLocationFromString(
@@ -62,3 +69,14 @@ function isWholeFileMatch(matches: RegExpExecArray): boolean {
     matches[5] === '0'
   );
 }
+
+/**
+ * Checks whether the file path is empty. For now, just check whether
+ * the file path is empty. If so, we do not want to render this location
+ * as a link.
+ *
+ * @param path A file path
+ */
+function isEmptyPath(path: string) {
+  return !path || path === '/';
+}

extensions/ql-vscode/src/extension.ts

@@ -408,7 +408,11 @@ async function activateWithInstalledDistribution(
         await showResultsForCompletedQuery(item, WebviewReveal.NotForced);
       } catch (e) {
         if (e instanceof UserCancellationException) {
-          helpers.showAndLogWarningMessage(e.message);
+          if (e.silent) {
+            logger.log(e.message);
+          } else {
+            helpers.showAndLogWarningMessage(e.message);
+          }
         } else if (e instanceof Error) {
           helpers.showAndLogErrorMessage(e.message);
         } else {

extensions/ql-vscode/src/interface.ts

@@ -270,7 +270,15 @@ export class InterfaceManager extends DisposableObject {
           await this.showPageOfInterpretedResults(msg.pageNumber);
         }
         else {
-          await this.showPageOfRawResults(msg.selectedTable, msg.pageNumber);
+          await this.showPageOfRawResults(
+            msg.selectedTable,
+            msg.pageNumber,
+            // When we are in an unsorted state, we guarantee that
+            // sortedResultsInfo doesn't have an entry for the current
+            // result set. Use this to determine whether or not we use
+            // the sorted bqrs file.
+            this._displayedQuery?.sortedResultsInfo.has(msg.selectedTable) || false
+          );
         }
         break;
       default:

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

@@ -34,7 +34,15 @@ export const tmpDirDisposal = {
   }
 };
 
-export class UserCancellationException extends Error { }
+export class UserCancellationException extends Error {
+  /**
+   * @param message The error message
+   * @param silent If silent is true, then this exception will avoid showing a warning message to the user.
+   */
+  constructor(message?: string, public readonly silent = false) {
+    super(message);
+  }
+}
 
 /**
  * A collection of evaluation-time information about a query,
@@ -307,7 +315,11 @@ async function checkDbschemeCompatibility(
 
 /**
  * Prompts the user to save `document` if it has unsaved changes.
- * Returns true if we should save changes.
+ *
+ * @param document The document to save.
+ *
+ * @returns true if we should save changes and false if we should continue without saving changes.
+ * @throws UserCancellationException if we should abort whatever operation triggered this prompt
  */
 async function promptUserToSaveChanges(document: vscode.TextDocument): Promise<boolean> {
   if (document.isDirty) {
@@ -317,9 +329,14 @@ async function promptUserToSaveChanges(document: vscode.TextDocument): Promise<b
     else {
       const yesItem = { title: 'Yes', isCloseAffordance: false };
       const alwaysItem = { title: 'Always Save', isCloseAffordance: false };
-      const noItem = { title: 'No', isCloseAffordance: true };
+      const noItem = { title: 'No (run anyway)', isCloseAffordance: false };
+      const cancelItem = { title: 'Cancel', isCloseAffordance: true };
       const message = 'Query file has unsaved changes. Save now?';
-      const chosenItem = await vscode.window.showInformationMessage(message, { modal: true }, yesItem, alwaysItem, noItem);
+      const chosenItem = await vscode.window.showInformationMessage(
+        message,
+        { modal: true },
+        yesItem, alwaysItem, noItem, cancelItem
+      );
 
       if (chosenItem === alwaysItem) {
         await config.AUTOSAVE_SETTING.updateValue(true, vscode.ConfigurationTarget.Workspace);
@@ -329,6 +346,10 @@ async function promptUserToSaveChanges(document: vscode.TextDocument): Promise<b
       if (chosenItem === yesItem) {
         return true;
       }
+
+      if (chosenItem === cancelItem) {
+        throw new UserCancellationException('Query run cancelled.', true);
+      }
     }
   }
   return false;

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

@@ -119,6 +119,26 @@ describe('interface-utils', () => {
       );
     });
 
+    it('should resolve a five-part location with an empty path', () => {
+      const mockDatabaseItem: DatabaseItem = ({
+        resolveSourceFile: sinon.stub().returns(vscode.Uri.parse('abc')),
+      } as unknown) as DatabaseItem;
+
+      expect(
+        tryResolveLocation(
+          {
+            t: LocationStyle.FivePart,
+            colStart: 1,
+            colEnd: 3,
+            lineStart: 4,
+            lineEnd: 5,
+            file: '',
+          },
+          mockDatabaseItem
+        )
+      ).to.be.undefined;
+    });
+
     it('should resolve a string location for whole file', () => {
       const mockDatabaseItem: DatabaseItem = ({
         resolveSourceFile: sinon.stub().returns(vscode.Uri.parse('abc')),

extensions/ql-vscode/syntaxes/ql.tmLanguage.yml

@@ -99,6 +99,12 @@ repository:
   comment-start:
     match: '// | /\*'
 
+  # A pattern that can start a run of whitespace or a comment.
+  # Commonly used as a negative lookahead in the `end` regex of a nonterminal, when searching for
+  # tokens that can't start a child of that nonterminal.
+  whitespace-or-comment-start:
+    match: '\s | $ | (?#comment-start)'
+
   # All tokens that can appear in any context.
   non-context-sensitive:
     patterns:
@@ -113,7 +119,7 @@ repository:
     name: keyword.operator.relational.ql
   
   comparison-operator:
-    match: '=|\!-'
+    match: '=|\!\='
     name: keyword.operator.comparison.ql
   
   arithmetic-operator:
@@ -610,6 +616,12 @@ repository:
     - include: '#import-directive'
     - include: '#import-as-clause'
     - include: '#module-declaration'
+    - include: '#newtype-declaration'
+    # See the comment on newtype-declaration for why we include these next three nonterminals at the
+    # module-member level instead of as part of the newtype-declaration.
+    - include: '#newtype-branch-name-with-prefix'
+    - include: '#predicate-parameter-list'
+    - include: '#predicate-body'
     - include: '#class-declaration'
     - include: '#select-clause'
     - include: '#predicate-or-field-declaration'
@@ -781,7 +793,7 @@ repository:
   bindingset-annotation:
     beginPattern: '#bindingset'
     # Ends after the next `]`, or when we encounter something other than a `[`.
-    end: '(?! \s | (?#comment-start) | \[ ) |
+    end: '(?! (?#whitespace-or-comment-start) | \[ ) |
           (?<=\])'
     name: meta.block.bindingset-annotation.ql
     patterns:
@@ -802,7 +814,7 @@ repository:
   language-annotation:
     beginPattern: '#language'
     # Ends after the next `]`, or when we encounter something other than a `[`.
-    end: '(?! \s | (?#comment-start) | \[ ) |
+    end: '(?! (?#whitespace-or-comment-start) | \[ ) |
           (?<=\])'
     name: meta.block.language-annotation.ql
     patterns:
@@ -824,7 +836,7 @@ repository:
   pragma-annotation:
     beginPattern: '#pragma'
     # Ends after the next `]`, or when we encounter something other than a `[`.
-    end: '(?! \s | (?#comment-start) | \[ ) |
+    end: '(?! (?#whitespace-or-comment-start) | \[ ) |
           (?<=\])'
     name: meta.block.pragma-annotation.ql
     patterns:
@@ -841,34 +853,53 @@ repository:
       name: storage.modifier.ql
 
   # The declaration of an IPA type.
+  # This only includes the `newtype` keyword and the identifier for the IPA type itself. The
+  # branches of the IPA type are modeled as separate nonterminals contained directly in the module
+  # body. This is kind of hacky, but without it, we don't seem to have a way to get TextMate to
+  # handle this:
+  # ```ql
+  # newtype TRoot =
+  #   TBranch1(int x) {
+  #     x = 5
+  #   } or
+  #   TBranch2()  // No body
+  #
+  # TOther getOther() { any() }
+  # ```
+  # If the branches are within the newtype declaration node, it's very hard to get the upper-id for
+  # the name of the IPA type to be included in the newtype declaration node, without also including
+  # the `TOther` upper-id in the declaration node.
   newtype-declaration:
     beginPattern: '#newtype'
-    # Ends when we see something other than one of:
-    # - An upper-id (branch name)
-    # - A comment
-    # - Whitespace
-    # - `=`
-    # - `(`
-    end: '(?! \s | (?#upper-id) | (?#comment-start) | \= | \( )'
-    name: meta.block.newtype.ql
-    patterns:
-    - include: '#non-context-sensitive'
-    - include: '#newtype-branch'
-
-  # The branch of an IPA type.
-  newtype-branch:
-    begin: '(?#upper-id)'
-    beginCaptures:
+    # We're expecting a newtype-declaration-without-keyword immediately after the `newtype` keyword,
+    # so end if we see anything other than the upper-id that starts it, or whitespace, or a comment.
+    # An upper-id can't start anything else at module scope, so once we see the rest of this
+    # newtype-declaration, whatever comes next should end this block.
+    end: '(?#upper-id)'
+    endCaptures:
       '0':
         name: entity.name.type.ql
-    # Ends after a `}`, or when we encounter something other than a `{`.
-    end: '(?<=\}) | (?! \s | (?#comment-start) | \{ )'
-    name: meta.block.newtype-branch.ql
+    name: meta.block.newtype-declaration.ql
+    patterns:
+    - include: '#non-context-sensitive'
+
+  # A branch of an IPA type, including just the `=` or `or` prefix and the name of the branch.
+  # The parameter list and body are separate nonterminals contained directly within the module body.
+  # See the comment for newtype-declaration for why.
+  newtype-branch-name-with-prefix:
+    begin: '\= | (?#or)'
+    beginCaptures:
+      '0':
+        patterns:
+        - include: '#or'
+        - include: '#comparison-operator'
+    end: '(?#upper-id)'
+    endCaptures:
+      '0':
+        name: entity.name.type.ql
+    name: meta.block.newtype-branch-name-with-prefix.ql
     patterns:
-    - include: '#predicate-body'
     - include: '#non-context-sensitive'
-    - match: '(?#upper-id)'
-      name: entity.name.type.ql
 
   # The declaration of a class, include an alias.
   class-declaration:

tsconfig.json

@@ -0,0 +1,3 @@
+{
+  "extends": "./extensions/ql-vscode/tsconfig.json"
+}