Deal with analysis messages that have links to locations (#1195)

Author: charisk

extensions/ql-vscode/src/pure/sarif-utils.ts

@@ -127,35 +127,49 @@ export function parseSarifLocation(
       userVisibleFile
     } as ParsedSarifLocation;
   } else {
-    const region = physicalLocation.region;
-    // We assume that the SARIF we're given always has startLine
-    // This is not mandated by the SARIF spec, but should be true of
-    // SARIF output by our own tools.
-    const startLine = region.startLine!;
-
-    // These defaults are from SARIF 2.1.0 spec, section 3.30.2, "Text Regions"
-    // https://docs.oasis-open.org/sarif/sarif/v2.1.0/cs01/sarif-v2.1.0-cs01.html#_Ref493492556
-    const endLine = region.endLine === undefined ? startLine : region.endLine;
-    const startColumn = region.startColumn === undefined ? 1 : region.startColumn;
-
-    // We also assume that our tools will always supply `endColumn` field, which is
-    // fortunate, since the SARIF spec says that it defaults to the end of the line, whose
-    // length we don't know at this point in the code.
-    //
-    // It is off by one with respect to the way vscode counts columns in selections.
-    const endColumn = region.endColumn! - 1;
+    const region = parseSarifRegion(physicalLocation.region);
 
     return {
       uri: effectiveLocation,
       userVisibleFile,
-      startLine,
-      startColumn,
-      endLine,
-      endColumn,
+      ...region
     };
   }
 }
 
+export function parseSarifRegion(
+  region: Sarif.Region
+): {
+  startLine: number,
+  endLine: number,
+  startColumn: number,
+  endColumn: number
+} {
+  // The SARIF we're given should have a startLine, but we
+  // fall back to 1, just in case something has gone wrong.
+  const startLine = region.startLine ?? 1;
+
+  // These defaults are from SARIF 2.1.0 spec, section 3.30.2, "Text Regions"
+  // https://docs.oasis-open.org/sarif/sarif/v2.1.0/cs01/sarif-v2.1.0-cs01.html#_Ref493492556
+  const endLine = region.endLine === undefined ? startLine : region.endLine;
+  const startColumn = region.startColumn === undefined ? 1 : region.startColumn;
+
+  // Our tools should always supply `endColumn` field, which is fortunate, since 
+  // the SARIF spec says that it defaults to the end of the line, whose
+  // length we don't know at this point in the code. We fall back to 1,
+  // just in case something has gone wrong.
+  //
+  // It is off by one with respect to the way vscode counts columns in selections.
+  const endColumn = (region.endColumn ?? 1) - 1;
+
+  return {
+    startLine,
+    startColumn,
+    endLine,
+    endColumn
+  };
+}
+
 export function isNoLocation(loc: ParsedSarifLocation): loc is NoLocation {
   return 'hint' in loc;
 }

extensions/ql-vscode/src/remote-queries/sample-data.ts

@@ -101,7 +101,35 @@ export const sampleRemoteQueryResult: RemoteQueryResult = {
 
 const createAnalysisInterpretedResults = (n: number) => Array(n).fill(
   {
-    message: 'This shell command depends on an uncontrolled [absolute path](1).',
+    message: {
+      tokens: [
+        {
+          t: 'text',
+          text: 'This shell command depends on an uncontrolled '
+        },
+        {
+          t: 'location',
+          text: 'absolute path',
+          location: {
+            filePath: 'npm-packages/meteor-installer/config.js',
+            codeSnippet: {
+              startLine: 33,
+              endLine: 37,
+              text: '\nconst meteorLocalFolder = \'.meteor\';\nconst meteorPath = path.resolve(rootPath, meteorLocalFolder);\n\nmodule.exports = {\n'
+            },
+            highlightedRegion: {
+              startLine: 35,
+              startColumn: 20,
+              endColumn: 61
+            }
+          }
+        },
+        {
+          t: 'text',
+          text: '.'
+        }
+      ]
+    },
     shortDescription: 'Shell command built from environment values',
     severity: 'Error',
     filePath: 'npm-packages/meteor-installer/config.js',