Tidy up how we display paths

Author: shati-patel

extensions/ql-vscode/src/remote-queries/remote-queries-markdown-generation.ts

@@ -89,12 +89,18 @@ function generateMarkdownForInterpretedResult(interpretedResult: AnalysisAlert,
       ...generateMarkdownForCodeSnippet(codeSnippet, language, highlightedRegion),
     );
   }
-  const alertMessageLines = buildMarkdownAlertMessage(interpretedResult, language);
-  lines.push(...alertMessageLines);
+  const alertMessage = generateMarkdownForAlertMessage(interpretedResult);
+  lines.push(alertMessage, '');
+
+  // If available, show paths
+  const hasPathResults = interpretedResult.codeFlows.length > 0;
+  if (hasPathResults) {
+    const pathLines = generateMarkdownForPathResults(interpretedResult, language);
+    lines.push(...pathLines);
+  }
 
   // Padding between results
   lines.push(
-    '',
     '----------------------------------------',
     '',
   );
@@ -142,50 +148,36 @@ function highlightCodeLines(
   return `${partiallyHighlightedLine.plainSection1}<strong>${partiallyHighlightedLine.highlightedSection}</strong>${partiallyHighlightedLine.plainSection2}`;
 }
 
-function buildMarkdownAlertMessage(
-  interpretedResult: AnalysisAlert,
-  language: string
-): string[] {
-  const hasPathResults = interpretedResult.codeFlows.length > 0;
-  if (hasPathResults) {
-    // For path-problem queries, the "alert message" is an expandable section containing the path results.
-    return buildMarkdownPathResults(interpretedResult, language);
-  } else {
-    let alertMessage = '';
-    // For regular problem queries (no paths), the alert message is just a message
-    // containing a link to the affected file.
-    for (const token of interpretedResult.message.tokens) {
-      if (token.t === 'text') {
-        alertMessage += token.text;
-      } else if (token.t === 'location') {
-        alertMessage += createMarkdownRemoteFileRef(
-          token.location.fileLink,
-          token.location.highlightedRegion?.startLine,
-          token.location.highlightedRegion?.endLine,
-          token.text
-        );
-      }
+function generateMarkdownForAlertMessage(
+  interpretedResult: AnalysisAlert
+): string {
+  let alertMessage = '';
+  for (const token of interpretedResult.message.tokens) {
+    if (token.t === 'text') {
+      alertMessage += token.text;
+    } else if (token.t === 'location') {
+      alertMessage += createMarkdownRemoteFileRef(
+        token.location.fileLink,
+        token.location.highlightedRegion?.startLine,
+        token.location.highlightedRegion?.endLine,
+        token.text
+      );
     }
-    // Italicize the alert message
-    return [`*${alertMessage}*`];
   }
+  // Italicize the alert message
+  return `*${alertMessage}*`;
 }
 
-function buildMarkdownPathResults(
+function generateMarkdownForPathResults(
   interpretedResult: AnalysisAlert,
   language: string
 ): MarkdownFile {
-  let alertMessage = '';
-  for (const token of interpretedResult.message.tokens) {
-    alertMessage += token.text;
-  }
   const pathLines: MarkdownFile = [];
-  pathLines.push('#### Paths', '');
   for (const codeFlow of interpretedResult.codeFlows) {
     const stepCount = codeFlow.threadFlows.length;
-    pathLines.push(`Path with ${stepCount} steps`);
-    let index = 1;
-    for (const threadFlow of codeFlow.threadFlows) {
+    pathLines.push(`#### Path with ${stepCount} steps`);
+    for (let i = 0; i < codeFlow.threadFlows.length; i++) {
+      const threadFlow = codeFlow.threadFlows[i];
       const link = createMarkdownRemoteFileRef(
         threadFlow.fileLink,
         threadFlow.highlightedRegion?.startLine,
@@ -198,11 +190,10 @@ function buildMarkdownPathResults(
       );
       // Indent the snippet to fit with the numbered list.
       const codeSnippetIndented = codeSnippet.map((line) => `    ${line}`);
-      pathLines.push(`${index}. ${link}`, ...codeSnippetIndented);
-      index++;
+      pathLines.push(`${i + 1}. ${link}`, ...codeSnippetIndented);
     }
   }
-  return buildExpandableMarkdownSection(`<i>${alertMessage}</i>`, pathLines);
+  return buildExpandableMarkdownSection('Show paths', pathLines);
 }
 
 /**

extensions/ql-vscode/test/pure-tests/remote-queries/markdown-generation/data/interpreted-results/path-problem/results-repo1.md

@@ -8,12 +8,12 @@
 }
 </code></pre>
 
-<details>
-<summary><i>This shell command depends on an uncontrolled absolute path.</i></summary>
+*This shell command depends on an uncontrolled [absolute path](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment.js#L4-L4).*
 
-#### Paths
+<details>
+<summary>Show paths</summary>
 
-Path with 5 steps
+#### Path with 5 steps
 1. [javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment.js](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment.js#L4-L4)
     <pre><code class="javascript">  path = require("path");
     function cleanupTemp() {
@@ -56,7 +56,6 @@ Path with 5 steps
 
 </details>
 
-
 ----------------------------------------
 
 [javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js#L6-L6)
@@ -68,12 +67,12 @@ Path with 5 steps
 	execa.shell('rm -rf ' + path.join(__dirname, "temp")); // NOT OK
 </code></pre>
 
-<details>
-<summary><i>This shell command depends on an uncontrolled absolute path.</i></summary>
+*This shell command depends on an uncontrolled [absolute path](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js#L6-L6).*
 
-#### Paths
+<details>
+<summary>Show paths</summary>
 
-Path with 3 steps
+#### Path with 3 steps
 1. [javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js#L6-L6)
     <pre><code class="javascript">(function() {
     	cp.execFileSync('rm',  ['-rf', path.join(__dirname, "temp")]); // GOOD
@@ -101,7 +100,6 @@ Path with 3 steps
 
 </details>
 
-
 ----------------------------------------
 
 [javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js#L8-L8)
@@ -113,12 +111,12 @@ Path with 3 steps
 
 </code></pre>
 
-<details>
-<summary><i>This shell command depends on an uncontrolled absolute path.</i></summary>
+*This shell command depends on an uncontrolled [absolute path](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js#L8-L8).*
 
-#### Paths
+<details>
+<summary>Show paths</summary>
 
-Path with 3 steps
+#### Path with 3 steps
 1. [javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js#L8-L8)
     <pre><code class="javascript">	cp.execSync('rm -rf ' + path.join(__dirname, "temp")); // BAD
 
@@ -146,7 +144,6 @@ Path with 3 steps
 
 </details>
 
-
 ----------------------------------------
 
 [javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js#L9-L9)
@@ -158,12 +155,12 @@ Path with 3 steps
 	const safe = "\"" + path.join(__dirname, "temp") + "\"";
 </code></pre>
 
-<details>
-<summary><i>This shell command depends on an uncontrolled absolute path.</i></summary>
+*This shell command depends on an uncontrolled [absolute path](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js#L9-L9).*
 
-#### Paths
+<details>
+<summary>Show paths</summary>
 
-Path with 3 steps
+#### Path with 3 steps
 1. [javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js](https://github.com/github/codeql/blob/48015e5a2e6202131f2d1062cc066dc33ed69a9b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js#L9-L9)
     <pre><code class="javascript">
     	execa.shell('rm -rf ' + path.join(__dirname, "temp")); // NOT OK
@@ -191,5 +188,4 @@ Path with 3 steps
 
 </details>
 
-
 ----------------------------------------

extensions/ql-vscode/test/pure-tests/remote-queries/markdown-generation/data/interpreted-results/path-problem/results-repo2.md

@@ -9,12 +9,12 @@
   }
 </code></pre>
 
-<details>
-<summary><i>This shell command depends on an uncontrolled absolute path.</i></summary>
+*This shell command depends on an uncontrolled [absolute path](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39-L39).*
 
-#### Paths
+<details>
+<summary>Show paths</summary>
 
-Path with 7 steps
+#### Path with 7 steps
 1. [npm-packages/meteor-installer/config.js](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39-L39)
     <pre><code class="javascript">
     const meteorLocalFolder = '.meteor';
@@ -86,5 +86,4 @@ Path with 7 steps
 
 </details>
 
-
 ----------------------------------------