From 6d75f7837586ccd43ac7c02971643d1c6be51071 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Mon, 28 Feb 2022 10:15:34 +0000 Subject: [PATCH 1/9] Add SARIF processing and basic alert rendering --- .../analyses-results-manager.ts | 28 ++-- .../src/remote-queries/sample-data.ts | 17 ++- .../src/remote-queries/sarif-processor.ts | 127 ++++++++++++++++++ .../remote-queries/shared/analysis-result.ts | 24 +++- .../view/AnalysisAlertResult.tsx | 98 ++++++++++++++ .../src/remote-queries/view/RemoteQueries.tsx | 13 +- .../src/remote-queries/view/remoteQueries.css | 18 +-- 7 files changed, 287 insertions(+), 38 deletions(-) create mode 100644 extensions/ql-vscode/src/remote-queries/sarif-processor.ts create mode 100644 extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx diff --git a/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts b/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts index 2b9f425dad0..7d0fa08e753 100644 --- a/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts @@ -6,9 +6,10 @@ import { Credentials } from '../authentication'; import { Logger } from '../logging'; import { downloadArtifactFromLink } from './gh-actions-api-client'; import { AnalysisSummary } from './shared/remote-query-result'; -import { AnalysisResults, QueryResult } from './shared/analysis-result'; +import { AnalysisResults, AnalysisAlert } from './shared/analysis-result'; import { UserCancellationException } from '../commandRunner'; import { sarifParser } from '../sarif-parser'; +import { processSarif } from './sarif-processor'; export class AnalysesResultsManager { // Store for the results of various analyses for each remote query. @@ -136,26 +137,15 @@ export class AnalysesResultsManager { void publishResults([...resultsForQuery]); } - private async readResults(filePath: string): Promise { - const queryResults: QueryResult[] = []; - + private async readResults(filePath: string): Promise { const sarifLog = await sarifParser(filePath); - // Read the sarif file and extract information that we want to display - // in the UI. For now we're only getting the message texts but we'll gradually - // extract more information based on the UX we want to build. - - sarifLog.runs?.forEach(run => { - run?.results?.forEach(result => { - if (result?.message?.text) { - queryResults.push({ - message: result.message.text - }); - } - }); - }); - - return queryResults; + const processedSarif = processSarif(sarifLog); + if (processedSarif.errors) { + void this.logger.log(`Error processing SARIF file: ${os.EOL}${processedSarif.errors.join(os.EOL)}`); + } + + return processedSarif.alerts; } private isAnalysisInMemory(analysis: AnalysisSummary): boolean { diff --git a/extensions/ql-vscode/src/remote-queries/sample-data.ts b/extensions/ql-vscode/src/remote-queries/sample-data.ts index d12f7f3d319..5323132e21e 100644 --- a/extensions/ql-vscode/src/remote-queries/sample-data.ts +++ b/extensions/ql-vscode/src/remote-queries/sample-data.ts @@ -98,7 +98,22 @@ export const sampleRemoteQueryResult: RemoteQueryResult = { }; -const createAnalysisResults = (n: number) => Array(n).fill({ 'message': 'Sample text' }); +const createAnalysisResults = (n: number) => Array(n).fill( + { + message: 'This shell command depends on an uncontrolled [absolute path](1).', + filePath: 'npm-packages/meteor-installer/config.js', + contextRegion: { + startLine: 253, + endLine: 257, + text: ' if (isWindows()) {\n //set for the current session and beyond\n child_process.execSync(`setx path "${meteorPath}/;%path%`);\n return;\n }\n', + }, + codeRegion: { + startLine: 255, + startColumn: 28, + endColumn: 62 + } + } +); export const sampleAnalysesResultsStage1: AnalysisResults[] = [ { diff --git a/extensions/ql-vscode/src/remote-queries/sarif-processor.ts b/extensions/ql-vscode/src/remote-queries/sarif-processor.ts new file mode 100644 index 00000000000..36ae3a154fb --- /dev/null +++ b/extensions/ql-vscode/src/remote-queries/sarif-processor.ts @@ -0,0 +1,127 @@ +import * as sarif from 'sarif'; + +import { AnalysisAlert, ResultSeverity } from './shared/analysis-result'; + +export class ProcessedSarif { + public constructor( + public readonly alerts: AnalysisAlert[], + public readonly errors: string[]) { } +} + +export function processSarif(sarifLog: sarif.Log): ProcessedSarif { + if (!sarifLog) { + return new ProcessedSarif([], ['No SARIF log was found']); + } + + if (!sarifLog.runs) { + return new ProcessedSarif([], ['No runs found in the SARIF file']); + } + + const errors = []; + const alerts = []; + + for (const run of sarifLog.runs) { + if (!run.results) { + errors.push('No results found in the SARIF run'); + continue; + } + + for (const result of run.results) { + const message = result.message?.text; + if (!message) { + errors.push('No message found in the SARIF result'); + continue; + } + + const severity = getSeverity(run, result); + + if (!result.locations) { + errors.push('No locations found in the SARIF result'); + continue; + } + + for (const location of result.locations) { + const contextRegion = location.physicalLocation?.contextRegion; + if (!contextRegion) { + errors.push('No context region found in the SARIF result location'); + continue; + } + if (contextRegion.startLine === undefined) { + errors.push('No start line set for a result context region'); + continue; + } + if (contextRegion.endLine === undefined) { + errors.push('No end line set for a result context region'); + continue; + } + if (!contextRegion.snippet?.text) { + errors.push('No text set for a result context region'); + continue; + } + + const codeRegion = location.physicalLocation?.region; + if (!codeRegion) { + errors.push('No code region found in the SARIF result location'); + continue; + } + if (codeRegion.startLine === undefined) { + errors.push('No start line set for a result code region'); + continue; + } + if (codeRegion.startColumn === undefined) { + errors.push('No start column set for a result code region'); + continue; + } + if (codeRegion.endColumn === undefined) { + errors.push('No end column set for a result code region'); + continue; + } + + const filePath = location.physicalLocation?.artifactLocation?.uri; + if (!filePath) { + errors.push('No file path found in the SARIF result location'); + continue; + } + + alerts.push({ + message, + filePath, + severity, + contextRegion: { + startLine: contextRegion.startLine, + endLine: contextRegion.endLine, + text: contextRegion.snippet.text + }, + codeRegion: { + startLine: codeRegion.startLine, + startColumn: codeRegion.startColumn, + endColumn: codeRegion.endColumn + } + }); + } + } + } + + return new ProcessedSarif(alerts, errors); +} + +function getSeverity(sarifRun: sarif.Run, result: sarif.Result): ResultSeverity { + const defaultSeverity = 'Warning'; + + const ruleId = result.ruleId; + if (!ruleId) { + return defaultSeverity; + } + + const rule = sarifRun.tool.driver.rules?.find(r => r.id === ruleId); + if (!rule) { + return defaultSeverity; + } + + const severity = rule.properties?.['problem.severity']; + if (!severity) { + return defaultSeverity; + } + + return severity.toLowerCase() === 'warning' ? 'Warning' : 'Error'; +} diff --git a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts index 07926cc26e5..bd5390993e1 100644 --- a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts +++ b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts @@ -3,9 +3,27 @@ export type AnalysisResultStatus = 'InProgress' | 'Completed' | 'Failed'; export interface AnalysisResults { nwo: string; status: AnalysisResultStatus; - results: QueryResult[]; + results: AnalysisAlert[]; } -export interface QueryResult { - message?: string; +export interface AnalysisAlert { + message: string; + severity: ResultSeverity; + filePath: string; + contextRegion: ContextRegion + codeRegion: CodeRegion } + +export interface ContextRegion { + startLine: number; + endLine: number; + text: string; +} + +export interface CodeRegion { + startLine: number; + startColumn: number; + endColumn: number; +} + +export type ResultSeverity = 'Warning' | 'Error'; diff --git a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx new file mode 100644 index 00000000000..5f7ec7ac681 --- /dev/null +++ b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx @@ -0,0 +1,98 @@ +import * as React from 'react'; +import styled from 'styled-components'; +import { Box, Link } from '@primer/react'; +import { AnalysisAlert, ResultSeverity } from '../shared/analysis-result'; + +const borderColor = 'var(--vscode-editor-snippetFinalTabstopHighlightBorder)'; +const warningColor = '#966C23'; + +const getSeverityColor = (severity: ResultSeverity) => { + return severity === 'Error' ? 'red' : warningColor; +}; + +const Container = styled.div` + font-family: ui-monospace, SFMono-Regular, SF Mono, Menlo, Consolas, Liberation Mono, monospace; +`; + +const TitleContainer = styled.div` + border: 0.1em solid ${borderColor}; + border-top-left-radius: 0.2em; + border-top-right-radius: 0.2em; + padding: 0.5em; +`; + +const CodeContainer = styled.div` + font-size: x-small; + border-left: 0.1em solid ${borderColor}; + border-right: 0.1em solid ${borderColor}; + border-bottom: 0.1em solid ${borderColor}; + border-bottom-left-radius: 0.2em; + border-bottom-right-radius: 0.2em; +`; + +const MessageText = styled.span<{ severity: ResultSeverity }>` + font-size: x-small; + color: ${props => `${getSeverityColor(props.severity)}`}; + padding-left: 0.5em; +`; + +const MessageContainer = styled.div` + padding-top: 0.5em; + padding-bottom: 0.5em; +`; + +const Message = ({ alert, currentLineNumber }: { + alert: AnalysisAlert, + currentLineNumber: number +}) => { + if (alert.codeRegion.startLine !== currentLineNumber) { + return <>; + } + return + + {alert.message} + + + ; +}; + +const AnalysisAlertResult = ({ alert }: { alert: AnalysisAlert }) => { + const code = alert.contextRegion.text + .split('\n') + .filter(line => line.replace('\n', '').length > 0); + + const startingLine = alert.contextRegion.startLine; + + return ( + + + {alert.filePath} + + + {code.map((line, index) => ( +
+ + {/* TODO: Replace the following with actual code snippet component */} + + + {startingLine + index} + + + {line} + + +
+ ))} + + + ); +}; + +export default AnalysisAlertResult; diff --git a/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx b/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx index 1baf345fc35..d5e95cf53eb 100644 --- a/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx @@ -17,6 +17,7 @@ import { AnalysisResults } from '../shared/analysis-result'; import DownloadSpinner from './DownloadSpinner'; import CollapsibleItem from './CollapsibleItem'; import { AlertIcon, CodeSquareIcon, FileCodeIcon, FileSymlinkFileIcon, RepoIcon, TerminalIcon } from '@primer/octicons-react'; +import AnalysisAlertResult from './AnalysisAlertResult'; const numOfReposInContractedMode = 10; @@ -220,7 +221,7 @@ const Summary = ({ analysesResults={analysesResults} /> } -
    +
      {queryResult.analysisSummaries.slice(0, numOfReposToShow).map((summary, i) =>
    • { return ( - {analysisResults.results.map((r, i) => (

      {r.message}

      ))} +
        + {analysisResults.results.map((r, i) => +
      • + + +
        • )} +
      ); }; @@ -288,7 +295,7 @@ const AnalysesResults = ({ analysesResults, totalResults }: { analysesResults: A -
        +
          {analysesResults.filter(a => a.results.length > 0).map(r =>
        • diff --git a/extensions/ql-vscode/src/remote-queries/view/remoteQueries.css b/extensions/ql-vscode/src/remote-queries/view/remoteQueries.css index 028876e0e82..dfc75a000a3 100644 --- a/extensions/ql-vscode/src/remote-queries/view/remoteQueries.css +++ b/extensions/ql-vscode/src/remote-queries/view/remoteQueries.css @@ -12,22 +12,10 @@ padding-top: 1.5em; } -.vscode-codeql__analysis-summaries-list { - list-style-type: none; - margin: 0; - padding: 0.5em 0 0 0; -} - .vscode-codeql__analysis-summaries-list-item { margin-top: 0.5em; } -.vscode-codeql__analyses-results-list { - list-style-type: none; - margin: 0; - padding: 0.5em 0 0 0; -} - .vscode-codeql__analyses-results-list-item { padding-top: 0.5em; } @@ -55,3 +43,9 @@ Liberation Mono, monospace; color: var(--vscode-editor-foreground); } + +.vscode-codeql__flat-list { + list-style-type: none; + margin: 0; + padding: 0.5em 0 0 0; +} From 6eed2426f1a3b80b20875ff7d0a5edf48fe16cbe Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Mon, 28 Feb 2022 14:53:26 +0000 Subject: [PATCH 2/9] Support 'recommendation' severity level --- .../ql-vscode/src/remote-queries/sarif-processor.ts | 11 ++++++++++- .../src/remote-queries/shared/analysis-result.ts | 2 +- .../src/remote-queries/view/AnalysisAlertResult.tsx | 9 ++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/sarif-processor.ts b/extensions/ql-vscode/src/remote-queries/sarif-processor.ts index 36ae3a154fb..8186f3c302a 100644 --- a/extensions/ql-vscode/src/remote-queries/sarif-processor.ts +++ b/extensions/ql-vscode/src/remote-queries/sarif-processor.ts @@ -123,5 +123,14 @@ function getSeverity(sarifRun: sarif.Run, result: sarif.Result): ResultSeverity return defaultSeverity; } - return severity.toLowerCase() === 'warning' ? 'Warning' : 'Error'; + switch (severity.toLowerCase()) { + case 'recommendation': + return 'Recommendation'; + case 'warning': + return 'Warning'; + case 'error': + return 'Error'; + } + + return defaultSeverity; } diff --git a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts index bd5390993e1..c86654431f5 100644 --- a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts +++ b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts @@ -26,4 +26,4 @@ export interface CodeRegion { endColumn: number; } -export type ResultSeverity = 'Warning' | 'Error'; +export type ResultSeverity = 'Recommendation' | 'Warning' | 'Error'; diff --git a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx index 5f7ec7ac681..94f48577f59 100644 --- a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx @@ -7,7 +7,14 @@ const borderColor = 'var(--vscode-editor-snippetFinalTabstopHighlightBorder)'; const warningColor = '#966C23'; const getSeverityColor = (severity: ResultSeverity) => { - return severity === 'Error' ? 'red' : warningColor; + switch (severity) { + case 'Recommendation': + return 'blue'; + case 'Warning': + return warningColor; + case 'Error': + return 'red'; + } }; const Container = styled.div` From fec0ed291f0dccce714a321c56a9a6e7a77d36ae Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Mon, 28 Feb 2022 15:50:27 +0000 Subject: [PATCH 3/9] Update sample data --- extensions/ql-vscode/src/remote-queries/sample-data.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/ql-vscode/src/remote-queries/sample-data.ts b/extensions/ql-vscode/src/remote-queries/sample-data.ts index 23526dfadbe..4724fe9ec4f 100644 --- a/extensions/ql-vscode/src/remote-queries/sample-data.ts +++ b/extensions/ql-vscode/src/remote-queries/sample-data.ts @@ -102,6 +102,7 @@ export const sampleRemoteQueryResult: RemoteQueryResult = { const createAnalysisResults = (n: number) => Array(n).fill( { message: 'This shell command depends on an uncontrolled [absolute path](1).', + severity: 'Error', filePath: 'npm-packages/meteor-installer/config.js', contextRegion: { startLine: 253, From 01915cbcf8cb0ac13de1a261d02610b573ab996b Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Mon, 28 Feb 2022 16:53:14 +0000 Subject: [PATCH 4/9] Set data to allow for multi-line highlighting --- extensions/ql-vscode/src/remote-queries/sarif-processor.ts | 1 + .../ql-vscode/src/remote-queries/shared/analysis-result.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/extensions/ql-vscode/src/remote-queries/sarif-processor.ts b/extensions/ql-vscode/src/remote-queries/sarif-processor.ts index 8186f3c302a..5d29c9168f8 100644 --- a/extensions/ql-vscode/src/remote-queries/sarif-processor.ts +++ b/extensions/ql-vscode/src/remote-queries/sarif-processor.ts @@ -95,6 +95,7 @@ export function processSarif(sarifLog: sarif.Log): ProcessedSarif { codeRegion: { startLine: codeRegion.startLine, startColumn: codeRegion.startColumn, + endLine: codeRegion.endLine, endColumn: codeRegion.endColumn } }); diff --git a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts index c86654431f5..1872b2e5964 100644 --- a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts +++ b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts @@ -23,6 +23,7 @@ export interface ContextRegion { export interface CodeRegion { startLine: number; startColumn: number; + endLine: number | undefined; endColumn: number; } From b6698945b29d14875d24c6ebbe46d4b1349c9624 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Tue, 1 Mar 2022 14:28:16 +0000 Subject: [PATCH 5/9] Some renames and lots of unit tests --- .../analyses-results-manager.ts | 4 +- .../src/remote-queries/sarif-processing.ts | 179 ++++++ .../src/remote-queries/sarif-processor.ts | 137 ----- .../remote-queries/shared/analysis-result.ts | 8 +- .../view/AnalysisAlertResult.tsx | 6 +- .../test/pure-tests/sarif-processing.test.ts | 580 ++++++++++++++++++ 6 files changed, 768 insertions(+), 146 deletions(-) create mode 100644 extensions/ql-vscode/src/remote-queries/sarif-processing.ts delete mode 100644 extensions/ql-vscode/src/remote-queries/sarif-processor.ts create mode 100644 extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts diff --git a/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts b/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts index 7d0fa08e753..d28e4ed27b3 100644 --- a/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts @@ -9,7 +9,7 @@ import { AnalysisSummary } from './shared/remote-query-result'; import { AnalysisResults, AnalysisAlert } from './shared/analysis-result'; import { UserCancellationException } from '../commandRunner'; import { sarifParser } from '../sarif-parser'; -import { processSarif } from './sarif-processor'; +import { extractAnalysisAlerts } from './sarif-processing'; export class AnalysesResultsManager { // Store for the results of various analyses for each remote query. @@ -140,7 +140,7 @@ export class AnalysesResultsManager { private async readResults(filePath: string): Promise { const sarifLog = await sarifParser(filePath); - const processedSarif = processSarif(sarifLog); + const processedSarif = extractAnalysisAlerts(sarifLog); if (processedSarif.errors) { void this.logger.log(`Error processing SARIF file: ${os.EOL}${processedSarif.errors.join(os.EOL)}`); } diff --git a/extensions/ql-vscode/src/remote-queries/sarif-processing.ts b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts new file mode 100644 index 00000000000..50dc73b7147 --- /dev/null +++ b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts @@ -0,0 +1,179 @@ +import * as sarif from 'sarif'; + +import { AnalysisAlert, ResultSeverity } from './shared/analysis-result'; + +const defaultSeverity = 'Warning'; + +export function extractAnalysisAlerts( + sarifLog: sarif.Log +): { + alerts: AnalysisAlert[], + errors: string[] +} { + if (!sarifLog) { + return { alerts: [], errors: ['No SARIF log was found'] }; + } + + if (!sarifLog.runs) { + return { alerts: [], errors: ['No runs found in the SARIF file'] }; + } + + const errors: string[] = []; + const alerts: AnalysisAlert[] = []; + + for (const run of sarifLog.runs) { + if (!run.results) { + errors.push('No results found in the SARIF run'); + continue; + } + + for (const result of run.results) { + const message = result.message?.text; + if (!message) { + errors.push('No message found in the SARIF result'); + continue; + } + + const severity = tryGetSeverity(run, result) || defaultSeverity; + + if (!result.locations) { + errors.push('No locations found in the SARIF result'); + continue; + } + + for (const location of result.locations) { + const contextRegion = location.physicalLocation?.contextRegion; + if (!contextRegion) { + errors.push('No context region found in the SARIF result location'); + continue; + } + if (contextRegion.startLine === undefined) { + errors.push('No start line set for a result context region'); + continue; + } + if (contextRegion.endLine === undefined) { + errors.push('No end line set for a result context region'); + continue; + } + if (!contextRegion.snippet?.text) { + errors.push('No text set for a result context region'); + continue; + } + + const region = location.physicalLocation?.region; + if (!region) { + errors.push('No region found in the SARIF result location'); + continue; + } + if (region.startLine === undefined) { + errors.push('No start line set for a result region'); + continue; + } + if (region.startColumn === undefined) { + errors.push('No start column set for a result region'); + continue; + } + if (region.endColumn === undefined) { + errors.push('No end column set for a result region'); + continue; + } + + const filePath = location.physicalLocation?.artifactLocation?.uri; + if (!filePath) { + errors.push('No file path found in the SARIF result location'); + continue; + } + + alerts.push({ + message, + filePath, + severity, + codeSnippet: { + startLine: contextRegion.startLine, + endLine: contextRegion.endLine, + text: contextRegion.snippet.text + }, + highlightedRegion: { + startLine: region.startLine, + startColumn: region.startColumn, + endLine: region.endLine, + endColumn: region.endColumn + } + }); + } + } + } + + return { alerts, errors }; +} + +export function tryGetSeverity( + sarifRun: sarif.Run, + result: sarif.Result +): ResultSeverity | undefined { + if (!sarifRun || !result) { + return undefined; + } + + const rule = tryGetRule(sarifRun, result); + if (!rule) { + return undefined; + } + + const severity = rule.properties?.['problem.severity']; + if (!severity) { + return undefined; + } + + switch (severity.toLowerCase()) { + case 'recommendation': + return 'Recommendation'; + case 'warning': + return 'Warning'; + case 'error': + return 'Error'; + } + + return undefined; +} + +export function tryGetRule( + sarifRun: sarif.Run, + result: sarif.Result +): sarif.ReportingDescriptor | undefined { + if (!sarifRun || !result) { + return undefined; + } + + const resultRule = result.rule; + if (!resultRule) { + return undefined; + } + + // The rule can found in two places: + // - Either in the run's tool driver tool component + // - Or in the run's tool extensions tool component + + const ruleId = resultRule.id; + if (ruleId) { + const rule = sarifRun.tool.driver.rules?.find(r => r.id === ruleId); + if (rule) { + return rule; + } + } + + const ruleIndex = resultRule.index; + if (ruleIndex != undefined) { + const toolComponentIndex = result.rule?.toolComponent?.index; + const toolExtensions = sarifRun.tool.extensions; + if (toolComponentIndex !== undefined && toolExtensions !== undefined) { + const toolComponent = toolExtensions[toolComponentIndex]; + if (toolComponent?.rules !== undefined) { + return toolComponent.rules[ruleIndex]; + } + } + } + + // Couldn't find the rule. + return undefined; +} diff --git a/extensions/ql-vscode/src/remote-queries/sarif-processor.ts b/extensions/ql-vscode/src/remote-queries/sarif-processor.ts deleted file mode 100644 index 5d29c9168f8..00000000000 --- a/extensions/ql-vscode/src/remote-queries/sarif-processor.ts +++ /dev/null @@ -1,137 +0,0 @@ -import * as sarif from 'sarif'; - -import { AnalysisAlert, ResultSeverity } from './shared/analysis-result'; - -export class ProcessedSarif { - public constructor( - public readonly alerts: AnalysisAlert[], - public readonly errors: string[]) { } -} - -export function processSarif(sarifLog: sarif.Log): ProcessedSarif { - if (!sarifLog) { - return new ProcessedSarif([], ['No SARIF log was found']); - } - - if (!sarifLog.runs) { - return new ProcessedSarif([], ['No runs found in the SARIF file']); - } - - const errors = []; - const alerts = []; - - for (const run of sarifLog.runs) { - if (!run.results) { - errors.push('No results found in the SARIF run'); - continue; - } - - for (const result of run.results) { - const message = result.message?.text; - if (!message) { - errors.push('No message found in the SARIF result'); - continue; - } - - const severity = getSeverity(run, result); - - if (!result.locations) { - errors.push('No locations found in the SARIF result'); - continue; - } - - for (const location of result.locations) { - const contextRegion = location.physicalLocation?.contextRegion; - if (!contextRegion) { - errors.push('No context region found in the SARIF result location'); - continue; - } - if (contextRegion.startLine === undefined) { - errors.push('No start line set for a result context region'); - continue; - } - if (contextRegion.endLine === undefined) { - errors.push('No end line set for a result context region'); - continue; - } - if (!contextRegion.snippet?.text) { - errors.push('No text set for a result context region'); - continue; - } - - const codeRegion = location.physicalLocation?.region; - if (!codeRegion) { - errors.push('No code region found in the SARIF result location'); - continue; - } - if (codeRegion.startLine === undefined) { - errors.push('No start line set for a result code region'); - continue; - } - if (codeRegion.startColumn === undefined) { - errors.push('No start column set for a result code region'); - continue; - } - if (codeRegion.endColumn === undefined) { - errors.push('No end column set for a result code region'); - continue; - } - - const filePath = location.physicalLocation?.artifactLocation?.uri; - if (!filePath) { - errors.push('No file path found in the SARIF result location'); - continue; - } - - alerts.push({ - message, - filePath, - severity, - contextRegion: { - startLine: contextRegion.startLine, - endLine: contextRegion.endLine, - text: contextRegion.snippet.text - }, - codeRegion: { - startLine: codeRegion.startLine, - startColumn: codeRegion.startColumn, - endLine: codeRegion.endLine, - endColumn: codeRegion.endColumn - } - }); - } - } - } - - return new ProcessedSarif(alerts, errors); -} - -function getSeverity(sarifRun: sarif.Run, result: sarif.Result): ResultSeverity { - const defaultSeverity = 'Warning'; - - const ruleId = result.ruleId; - if (!ruleId) { - return defaultSeverity; - } - - const rule = sarifRun.tool.driver.rules?.find(r => r.id === ruleId); - if (!rule) { - return defaultSeverity; - } - - const severity = rule.properties?.['problem.severity']; - if (!severity) { - return defaultSeverity; - } - - switch (severity.toLowerCase()) { - case 'recommendation': - return 'Recommendation'; - case 'warning': - return 'Warning'; - case 'error': - return 'Error'; - } - - return defaultSeverity; -} diff --git a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts index 1872b2e5964..e448e757e37 100644 --- a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts +++ b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts @@ -10,17 +10,17 @@ export interface AnalysisAlert { message: string; severity: ResultSeverity; filePath: string; - contextRegion: ContextRegion - codeRegion: CodeRegion + codeSnippet: CodeSnippet + highlightedRegion: HighligtedRegion } -export interface ContextRegion { +export interface CodeSnippet { startLine: number; endLine: number; text: string; } -export interface CodeRegion { +export interface HighligtedRegion { startLine: number; startColumn: number; endLine: number | undefined; diff --git a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx index 94f48577f59..5b45af156c9 100644 --- a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx @@ -52,7 +52,7 @@ const Message = ({ alert, currentLineNumber }: { alert: AnalysisAlert, currentLineNumber: number }) => { - if (alert.codeRegion.startLine !== currentLineNumber) { + if (alert.highlightedRegion.startLine !== currentLineNumber) { return <>; } return @@ -71,11 +71,11 @@ const Message = ({ alert, currentLineNumber }: { }; const AnalysisAlertResult = ({ alert }: { alert: AnalysisAlert }) => { - const code = alert.contextRegion.text + const code = alert.codeSnippet.text .split('\n') .filter(line => line.replace('\n', '').length > 0); - const startingLine = alert.contextRegion.startLine; + const startingLine = alert.codeSnippet.startLine; return ( diff --git a/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts b/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts new file mode 100644 index 00000000000..61cd0a6ca5d --- /dev/null +++ b/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts @@ -0,0 +1,580 @@ +import 'vscode-test'; +import 'mocha'; +import * as chaiAsPromised from 'chai-as-promised'; +import * as chai from 'chai'; +import * as sarif from 'sarif'; +import { extractAnalysisAlerts, tryGetRule, tryGetSeverity } from '../../src/remote-queries/sarif-processing'; + +chai.use(chaiAsPromised); +const expect = chai.expect; + +describe('SARIF processing', () => { + describe('tryGetRule', () => { + describe('Using the tool driver', () => { + it('should return undefined if no rule has been set on the result', () => { + const result = { + message: 'msg' + // Rule is missing here. + } as sarif.Result; + + const sarifRun = { + results: [result] + } as sarif.Run; + + const rule = tryGetRule(sarifRun, result); + + expect(rule).to.be.undefined; + }); + + it('should return undefined if rule missing from tool driver', () => { + const result = { + message: 'msg', + rule: { + id: 'NonExistentRule' + } + } as sarif.Result; + + const sarifRun = { + results: [result], + tool: { + driver: { + rules: [ + // No rule with id 'NonExistentRule' is set here. + { + id: 'A', + }, + { + id: 'B' + } + ] + } + } + } as sarif.Run; + + const rule = tryGetRule(sarifRun, result); + + expect(rule).to.be.undefined; + }); + + it('should return rule if it has been set on the tool driver', () => { + const result = { + message: 'msg', + rule: { + id: 'B' + } + } as sarif.Result; + + const sarifRun = { + results: [result], + tool: { + driver: { + rules: [ + { + id: 'A', + }, + result.rule + ] + } + } + } as sarif.Run; + + const rule = tryGetRule(sarifRun, result); + + expect(rule).to.be.ok; + expect(rule!.id).to.equal(result!.rule!.id); + }); + }); + + describe('Using the tool extensions', () => { + it('should return undefined if rule index not set', () => { + const result = { + message: 'msg', + rule: { + // The rule index should be set here. + toolComponent: { + index: 1 + } + } + } as sarif.Result; + + const sarifRun = { + results: [result], + tool: { + extensions: [ + { + name: 'foo', + rules: [ + { + id: 'A', + }, + { + id: 'B' + } + ] + }, + { + name: 'bar', + rules: [ + { + id: 'C', + }, + { + id: 'D' + } + ] + } + ] + } + } as sarif.Run; + + const rule = tryGetRule(sarifRun, result); + + expect(rule).to.be.undefined; + }); + + it('should return undefined if tool component index not set', () => { + const result = { + message: 'msg', + rule: { + index: 1, + toolComponent: { + // The tool component index should be set here. + } + } + } as sarif.Result; + + const sarifRun = { + results: [result], + tool: { + extensions: [ + { + name: 'foo', + rules: [ + { + id: 'A', + }, + { + id: 'B' + } + ] + }, + { + name: 'bar', + rules: [ + { + id: 'C', + }, + { + id: 'D' + } + ] + } + ] + } + } as sarif.Run; + + const rule = tryGetRule(sarifRun, result); + + expect(rule).to.be.undefined; + }); + + it('should return undefined if tool extensions not set', () => { + const result = { + message: 'msg', + rule: { + index: 1, + toolComponent: { + index: 1 + } + } + } as sarif.Result; + + const sarifRun = { + results: [result], + tool: { + // Extensions should be set here. + } + } as sarif.Run; + + const rule = tryGetRule(sarifRun, result); + + expect(rule).to.be.undefined; + }); + + it('should return undefined if tool extensions do not contain index', () => { + const result = { + message: 'msg', + rule: { + index: 1, + toolComponent: { + index: 1 + } + } + } as sarif.Result; + + const sarifRun = { + results: [result], + tool: { + extensions: [ + { + name: 'foo', + rules: [ + { + id: 'A', + }, + { + id: 'B' + } + ] + } + // There should be one more extension here (index 1). + ] + } + } as sarif.Run; + + const rule = tryGetRule(sarifRun, result); + + expect(rule).to.be.undefined; + }); + + it('should return rule if all information is defined', () => { + const result = { + message: 'msg', + ruleIndex: 1, + rule: { + index: 1, + toolComponent: { + index: 1 + } + } + } as sarif.Result; + + const sarifRun = { + results: [result], + tool: { + extensions: [ + { + name: 'foo', + rules: [ + { + id: 'A', + }, + { + id: 'B' + } + ] + }, + { + name: 'bar', + rules: [ + { + id: 'C', + }, + { + id: 'D', + } + ] + } + ] + } + } as sarif.Run; + + const rule = tryGetRule(sarifRun, result); + + expect(rule).to.be.ok; + expect(rule!.id).to.equal('D'); + }); + }); + }); + + describe('tryGetSeverity', () => { + it('should return undefined if no rule found', () => { + const result = { + // The rule is missing here. + message: 'msg' + } as sarif.Result; + + const sarifRun = { + results: [result] + } as sarif.Run; + + const severity = tryGetSeverity(sarifRun, result); + expect(severity).to.be.undefined; + }); + + it('should return undefined if severity not set on rule', () => { + const result = { + message: 'msg', + rule: { + id: 'A' + } + } as sarif.Result; + + const sarifRun = { + results: [result], + tool: { + driver: { + rules: [ + { + id: 'A', + properties: { + // Severity not set + } + }, + result.rule + ] + } + } + } as sarif.Run; + + const severity = tryGetSeverity(sarifRun, result); + expect(severity).to.be.undefined; + }); + + const severityMap = { + recommendation: 'Recommendation', + warning: 'Warning', + error: 'Error' + }; + + Object.entries(severityMap).forEach(([sarifSeverity, parsedSeverity]) => { + it(`should get ${parsedSeverity} severity`, () => { + const result = { + message: 'msg', + rule: { + id: 'A' + } + } as sarif.Result; + + const sarifRun = { + results: [result], + tool: { + driver: { + rules: [ + { + id: 'A', + properties: { + 'problem.severity': sarifSeverity + } + }, + result.rule + ] + } + } + } as sarif.Run; + + const severity = tryGetSeverity(sarifRun, result); + expect(severity).to.equal(parsedSeverity); + }); + }); + + }); + + describe('extractAnalysisAlerts', () => { + it('should return an error if no runs found in the SARIF', () => { + const sarif = { + // Runs are missing here. + } as sarif.Log; + + const result = extractAnalysisAlerts(sarif); + + expect(result).to.be.ok; + expect(result.errors.length).to.equal(1); + expect(result.errors[0]).to.equal('No runs found in the SARIF file'); + }); + + it('should return errors for runs that have no results', () => { + const sarif = { + runs: [ + { + results: [] + }, + { + // Results are missing here. + } + ] + } as sarif.Log; + + const result = extractAnalysisAlerts(sarif); + + expect(result).to.be.ok; + expect(result.errors.length).to.equal(1); + expect(result.errors[0]).to.equal('No results found in the SARIF run'); + }); + + it('should return errors for results that have no message', () => { + const sarif = buildValidSarifLog(); + sarif.runs![0]!.results![0]!.message.text = undefined; + + const result = extractAnalysisAlerts(sarif); + + expect(result).to.be.ok; + expect(result.errors.length).to.equal(1); + expect(result.errors[0]).to.equal('No message found in the SARIF result'); + }); + + it('should return errors for result locations with no context region', () => { + const sarif = buildValidSarifLog(); + sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.contextRegion = undefined; + + const result = extractAnalysisAlerts(sarif); + + expect(result).to.be.ok; + expect(result.errors.length).to.equal(1); + expect(result.errors[0]).to.equal('No context region found in the SARIF result location'); + }); + + it('should return errors for result locations with no region', () => { + const sarif = buildValidSarifLog(); + sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.region = undefined; + + const result = extractAnalysisAlerts(sarif); + + expect(result).to.be.ok; + expect(result.errors.length).to.equal(1); + expect(result.errors[0]).to.equal('No region found in the SARIF result location'); + }); + + it('should return errors for result locations with no physical location', () => { + const sarif = buildValidSarifLog(); + sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.artifactLocation = undefined; + + const result = extractAnalysisAlerts(sarif); + + expect(result).to.be.ok; + expect(result.errors.length).to.equal(1); + expect(result.errors[0]).to.equal('No file path found in the SARIF result location'); + }); + + it('should return results for all alerts', () => { + const sarif = { + version: '0.0.1' as sarif.Log.version, + runs: [ + { + results: [ + { + message: { + text: 'msg1' + }, + locations: [ + { + physicalLocation: { + contextRegion: { + startLine: 10, + endLine: 12, + snippet: { + text: 'foo' + } + }, + region: { + startLine: 10, + startColumn: 1, + endColumn: 3 + }, + artifactLocation: { + uri: 'foo.js' + } + } + }, + { + physicalLocation: { + contextRegion: { + startLine: 10, + endLine: 12, + snippet: { + text: 'bar' + } + }, + region: { + startLine: 10, + startColumn: 1, + endColumn: 3 + }, + artifactLocation: { + uri: 'bar.js' + } + } + } + ] + }, + { + message: { + text: 'msg2' + }, + locations: [ + { + physicalLocation: { + contextRegion: { + startLine: 10, + endLine: 12, + snippet: { + text: 'baz' + } + }, + region: { + startLine: 10, + startColumn: 1, + endColumn: 3 + }, + artifactLocation: { + uri: 'baz.js' + } + } + } + ] + } + ] + } + ] + } as sarif.Log; + + const result = extractAnalysisAlerts(sarif); + expect(result).to.be.ok; + expect(result.errors.length).to.equal(0); + expect(result.alerts.length).to.equal(3); + expect(result.alerts.find(a => a.message === 'msg1' && a.codeSnippet.text === 'foo')).to.be.ok; + expect(result.alerts.find(a => a.message === 'msg1' && a.codeSnippet.text === 'bar')).to.be.ok; + expect(result.alerts.find(a => a.message === 'msg2' && a.codeSnippet.text === 'baz')).to.be.ok; + expect(result.alerts.every(a => a.severity === 'Warning')).to.be.true; + }); + + }); + + function buildValidSarifLog(): sarif.Log { + return { + version: '0.0.1' as sarif.Log.version, + runs: [ + { + results: [ + { + message: { + text: 'msg' + }, + locations: [ + { + physicalLocation: { + contextRegion: { + startLine: 10, + endLine: 12, + snippet: { + text: 'Foo' + } + }, + region: { + startLine: 10, + startColumn: 1, + endColumn: 3 + }, + artifactLocation: { + uri: 'foo.js' + } + } + } + ] + } + ] + } + ] + } as sarif.Log; + } +}); From 43a69607a84dc6551df4ec78871fb3063bfc52b6 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Tue, 1 Mar 2022 15:36:46 +0000 Subject: [PATCH 6/9] Add unstaged file --- extensions/ql-vscode/src/remote-queries/sample-data.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/sample-data.ts b/extensions/ql-vscode/src/remote-queries/sample-data.ts index 4724fe9ec4f..7ae1093a3eb 100644 --- a/extensions/ql-vscode/src/remote-queries/sample-data.ts +++ b/extensions/ql-vscode/src/remote-queries/sample-data.ts @@ -104,12 +104,12 @@ const createAnalysisResults = (n: number) => Array(n).fill( message: 'This shell command depends on an uncontrolled [absolute path](1).', severity: 'Error', filePath: 'npm-packages/meteor-installer/config.js', - contextRegion: { + codeSnippet: { startLine: 253, endLine: 257, text: ' if (isWindows()) {\n //set for the current session and beyond\n child_process.execSync(`setx path "${meteorPath}/;%path%`);\n return;\n }\n', }, - codeRegion: { + highlightedRegion: { startLine: 255, startColumn: 28, endColumn: 62 From 408c0252cf79c65b84a80dadfc51a91960f3cb8f Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Tue, 1 Mar 2022 16:41:04 +0000 Subject: [PATCH 7/9] Fix typo --- .../ql-vscode/src/remote-queries/shared/analysis-result.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts index e448e757e37..061f3aae3ae 100644 --- a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts +++ b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts @@ -11,7 +11,7 @@ export interface AnalysisAlert { severity: ResultSeverity; filePath: string; codeSnippet: CodeSnippet - highlightedRegion: HighligtedRegion + highlightedRegion: HighlightedRegion } export interface CodeSnippet { @@ -20,7 +20,7 @@ export interface CodeSnippet { text: string; } -export interface HighligtedRegion { +export interface HighlightedRegion { startLine: number; startColumn: number; endLine: number | undefined; From 6ab61c093519d243c4c778934aa132e55c36c022 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Tue, 1 Mar 2022 17:13:33 +0000 Subject: [PATCH 8/9] Make analysis results code snippets prettier --- .../view/AnalysisAlertResult.tsx | 80 +++++++++++++++++-- 1 file changed, 73 insertions(+), 7 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx index 5b45af156c9..4312e6e5d01 100644 --- a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx @@ -1,10 +1,11 @@ import * as React from 'react'; import styled from 'styled-components'; import { Box, Link } from '@primer/react'; -import { AnalysisAlert, ResultSeverity } from '../shared/analysis-result'; +import { AnalysisAlert, HighlightedRegion, ResultSeverity } from '../shared/analysis-result'; const borderColor = 'var(--vscode-editor-snippetFinalTabstopHighlightBorder)'; const warningColor = '#966C23'; +const highlightColor = '#534425'; const getSeverityColor = (severity: ResultSeverity) => { switch (severity) { @@ -35,11 +36,13 @@ const CodeContainer = styled.div` border-bottom: 0.1em solid ${borderColor}; border-bottom-left-radius: 0.2em; border-bottom-right-radius: 0.2em; + padding-top: 1em; + padding-bottom: 1em; `; const MessageText = styled.span<{ severity: ResultSeverity }>` font-size: x-small; - color: ${props => `${getSeverityColor(props.severity)}`}; + color: ${props => getSeverityColor(props.severity)}; padding-left: 0.5em; `; @@ -70,6 +73,54 @@ const Message = ({ alert, currentLineNumber }: { ; }; +const replaceSpaceChar = (text: string) => text.replaceAll(' ', '\u00a0'); + +const PlainLine = ({ text }: { text: string }) => { + return {replaceSpaceChar(text)}; +}; + +const HighlightedLine = ({ text }: { text: string }) => { + return {replaceSpaceChar(text)}; +}; + +const shouldHighlightLine = (lineNumber: number, highlightedRegion: HighlightedRegion) => { + if (lineNumber < highlightedRegion.startLine) { + return false; + } + + if (highlightedRegion.endLine) { + return lineNumber <= highlightedRegion.endLine; + } + + return true; +}; + +const CodeLine = ({ + line, + lineNumber, + highlightedRegion +}: { + line: string, + lineNumber: number, + highlightedRegion: HighlightedRegion +}) => { + if (!shouldHighlightLine(lineNumber, highlightedRegion)) { + return ; + } + + const section1 = line.substring(0, highlightedRegion.startColumn - 1); + const section2 = line.substring(highlightedRegion.startColumn - 1, highlightedRegion.endColumn - 1); + const section3 = line.substring(highlightedRegion.endColumn - 1, line.length); + + return ( + <> + + + + + ); +}; + const AnalysisAlertResult = ({ alert }: { alert: AnalysisAlert }) => { const code = alert.codeSnippet.text .split('\n') @@ -86,13 +137,28 @@ const AnalysisAlertResult = ({ alert }: { alert: AnalysisAlert }) => { {code.map((line, index) => (
          - {/* TODO: Replace the following with actual code snippet component */} - - + + {startingLine + index} - - {line} + +
          From 94bcca93f41e1511c9a4f998599a184289ff0736 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Wed, 2 Mar 2022 09:34:56 +0000 Subject: [PATCH 9/9] Add some basic validation --- .../src/remote-queries/sarif-processing.ts | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/sarif-processing.ts b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts index 50dc73b7147..ded7b001826 100644 --- a/extensions/ql-vscode/src/remote-queries/sarif-processing.ts +++ b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts @@ -84,7 +84,7 @@ export function extractAnalysisAlerts( continue; } - alerts.push({ + const analysisAlert = { message, filePath, severity, @@ -99,7 +99,15 @@ export function extractAnalysisAlerts( endLine: region.endLine, endColumn: region.endColumn } - }); + }; + + const validationErrors = getAlertValidationErrors(analysisAlert); + if (validationErrors.length > 0) { + errors.push(...validationErrors); + continue; + } + + alerts.push(analysisAlert); } } } @@ -177,3 +185,19 @@ export function tryGetRule( // Couldn't find the rule. return undefined; } + +function getAlertValidationErrors(alert: AnalysisAlert): string[] { + const errors = []; + + if (alert.codeSnippet.startLine > alert.codeSnippet.endLine) { + errors.push('The code snippet start line is greater than the end line'); + } + + const highlightedRegion = alert.highlightedRegion; + if (highlightedRegion.endLine === highlightedRegion.startLine && + highlightedRegion.endColumn < highlightedRegion.startColumn) { + errors.push('The highlighted region end column is greater than the start column'); + } + + return errors; +}