Tidy-up and address review comments

Author: shati-patel

extensions/ql-vscode/src/pure/location-link-utils.ts

@@ -13,17 +13,3 @@ export function createRemoteFileRef(
     return `${fileLink.fileLinkPrefix}/${fileLink.filePath}`;
   }
 }
-
-/**
- * Creates a markdown link to a remote file.
- * If the "link text" is not provided, we use the file path.
- */
-export function createMarkdownRemoteFileRef(
-  fileLink: FileLink,
-  startLine?: number,
-  endLine?: number,
-  linkText?: string,
-): string {
-  const markdownLink = `[${linkText || fileLink.filePath}](${createRemoteFileRef(fileLink, startLine, endLine)})`;
-  return markdownLink;
-}

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

@@ -1,14 +1,18 @@
-import { createMarkdownRemoteFileRef } from '../pure/location-link-utils';
+import { createRemoteFileRef } from '../pure/location-link-utils';
 import { RemoteQuery } from './remote-query';
-import { AnalysisAlert, AnalysisResults } from './shared/analysis-result';
+import { AnalysisAlert, AnalysisResults, FileLink } from './shared/analysis-result';
 
 // Each array item is a line of the markdown file.
 export type MarkdownFile = string[];
 
+/**
+ * Generates markdown files with variant analysis results.
+ */
 export function generateMarkdown(query: RemoteQuery, analysesResults: AnalysisResults[]): MarkdownFile[] {
   const files: MarkdownFile[] = [];
   for (const analysisResult of analysesResults) {
     if (analysisResult.interpretedResults.length === 0) {
+      // TODO: We'll add support for non-interpreted results later.
       continue;
     }
     const lines = [
@@ -31,17 +35,15 @@ function generateMarkdownForInterpretedResult(interpretedResult: AnalysisAlert,
     interpretedResult.fileLink,
     interpretedResult.highlightedRegion?.startLine,
     interpretedResult.highlightedRegion?.endLine
-  ), '');
+  ));
+  lines.push('');
   const codeSnippet = interpretedResult.codeSnippet?.text;
   if (codeSnippet) {
     lines.push(
-      `\`\`\`${language}`,
-      ...codeSnippet.split('\n'),
-      '```',
-      ''
+      ...generateMarkdownForCodeSnippet(codeSnippet, language),
     );
   }
-  const alertMessage = buildAlertMessage(interpretedResult);
+  const alertMessage = buildMarkdownAlertMessage(interpretedResult);
   lines.push(alertMessage);
 
   // Padding between results
@@ -53,7 +55,18 @@ function generateMarkdownForInterpretedResult(interpretedResult: AnalysisAlert,
   return lines;
 }
 
-function buildAlertMessage(interpretedResult: AnalysisAlert): string {
+function generateMarkdownForCodeSnippet(codeSnippet: string, language: string): MarkdownFile {
+  const lines: MarkdownFile = [];
+  lines.push(
+    `\`\`\`${language}`,
+    ...codeSnippet.split('\n'),
+    '```',
+  );
+  lines.push('');
+  return lines;
+}
+
+function buildMarkdownAlertMessage(interpretedResult: AnalysisAlert): string {
   let alertMessage = '';
   for (const token of interpretedResult.message.tokens) {
     if (token.t === 'text') {
@@ -67,5 +80,20 @@ function buildAlertMessage(interpretedResult: AnalysisAlert): string {
       );
     }
   }
-  return alertMessage;
+  // Italicize the alert message
+  return `*${alertMessage}*`;
+}
+
+/**
+ * Creates a markdown link to a remote file.
+ * If the "link text" is not provided, we use the file path.
+ */
+export function createMarkdownRemoteFileRef(
+  fileLink: FileLink,
+  startLine?: number,
+  endLine?: number,
+  linkText?: string,
+): string {
+  const markdownLink = `[${linkText || fileLink.filePath}](${createRemoteFileRef(fileLink, startLine, endLine)})`;
+  return markdownLink;
 }

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

@@ -10,7 +10,7 @@ function cleanupTemp() {
 
 ```
 
-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).
+*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).*
 
 ----------------------------------------
 
@@ -25,7 +25,7 @@ This shell command depends on an uncontrolled [absolute path](https://github.com
 
 ```
 
-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).
+*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).*
 
 ----------------------------------------
 
@@ -40,7 +40,7 @@ This shell command depends on an uncontrolled [absolute path](https://github.com
 
 ```
 
-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).
+*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).*
 
 ----------------------------------------
 
@@ -55,6 +55,6 @@ This shell command depends on an uncontrolled [absolute path](https://github.com
 
 ```
 
-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).
+*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).*
 
 ----------------------------------------

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

@@ -11,6 +11,6 @@
 
 ```
 
-This shell command depends on an uncontrolled [absolute path](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39-L39).
+*This shell command depends on an uncontrolled [absolute path](https://github.com/meteor/meteor/blob/73b538fe201cbfe89dd0c709689023f9b3eab1ec/npm-packages/meteor-installer/config.js#L39-L39).*
 
 ----------------------------------------

extensions/ql-vscode/test/pure-tests/remote-queries/markdown-generation/interpreted-results/markdown-generation.test.ts

@@ -20,11 +20,20 @@ describe('markdown generation', async function() {
     const markdownFile1 = markdownFiles[0]; // results for github/codeql repo
     const markdownFile2 = markdownFiles[1]; // results for meteor/meteor repo
 
-    const expectedTestOutput1 = await fs.readFile(path.join(__dirname, 'data/results-repo1.md'), 'utf8');
-    const expectedTestOutput2 = await fs.readFile(path.join(__dirname, 'data/results-repo2.md'), 'utf8');
+    const expectedTestOutput1 = await readTestOutputFile('data/results-repo1.md');
+    const expectedTestOutput2 = await readTestOutputFile('data/results-repo2.md');
 
     // Check that markdown output is correct, after making line endings consistent
-    expect(markdownFile1.join('\n')).to.equal(expectedTestOutput1.replace(/\r?\n/g, '\n'));
-    expect(markdownFile2.join('\n')).to.equal(expectedTestOutput2.replace(/\r?\n/g, '\n'));
+    expect(markdownFile1.join('\n')).to.equal(expectedTestOutput1);
+    expect(markdownFile2.join('\n')).to.equal(expectedTestOutput2);
   });
 });
+
+/**
+ * Reads a test output file and returns it as a string.
+ * Replaces line endings with '\n' for consistency across operating systems.
+ */
+async function readTestOutputFile(relativePath: string): Promise<string> {
+  const file = await fs.readFile(path.join(__dirname, relativePath), 'utf8');
+  return file.replace(/\r?\n/g, '\n');
+}