Skip to content

Commit

Permalink
fix: inter-file code flow in diagnostics [ROAD-864] (#214)
Browse files Browse the repository at this point in the history
  • Loading branch information
michelkaporin authored Apr 25, 2022
1 parent 55e1dfb commit 2449c68
Show file tree
Hide file tree
Showing 12 changed files with 260 additions and 84 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- "Error: Cannot get password" appearing during retrieval of the token from secret
storage.
- Cached Snyk Learn links being opened when clicking on "Learn about this vulnerability".
- Snyk Code inter-file issues linking only to the main file where issue occurs.

## [1.2.15]

Expand Down
17 changes: 13 additions & 4 deletions src/snyk/snykCode/analyzer/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Uri,
} from '../../common/vscode/types';
import { IUriAdapter } from '../../common/vscode/uri';
import { IVSCodeWorkspace } from '../../common/vscode/workspace';
import {
DIAGNOSTICS_CODE_QUALITY_COLLECTION_NAME,
DIAGNOSTICS_CODE_SECURITY_COLLECTION_NAME,
Expand Down Expand Up @@ -52,9 +53,10 @@ class SnykCodeAnalyzer implements ISnykCodeAnalyzer {
private readonly diagnosticSuggestion = new Map<Diagnostic, ICodeSuggestion>();

public constructor(
readonly logger: ILog,
readonly languages: IVSCodeLanguages,
readonly analytics: IAnalytics,
private readonly logger: ILog,
private readonly languages: IVSCodeLanguages,
private readonly workspace: IVSCodeWorkspace,
private readonly analytics: IAnalytics,
private readonly errorHandler: ISnykCodeErrorHandler,
private readonly uriAdapter: IUriAdapter,
private readonly configuration: IConfiguration,
Expand Down Expand Up @@ -141,7 +143,14 @@ class SnykCodeAnalyzer implements ISnykCodeAnalyzer {
: DIAGNOSTICS_CODE_QUALITY_COLLECTION_NAME,
// issues markers can be in issuesPositions as prop 'markers',
...(issuePositions.markers && {
relatedInformation: createIssueRelatedInformation(issuePositions.markers, fileUri, message, this.languages),
relatedInformation: createIssueRelatedInformation(
issuePositions.markers,
fileUri.path,
message,
this.languages,
this.workspace,
this.uriAdapter,
),
}),
};
}
Expand Down
11 changes: 10 additions & 1 deletion src/snyk/snykCode/codeService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,15 @@ export class SnykCodeService extends AnalysisStatusProvider implements ISnykCode
private readonly learnService: LearnService,
) {
super();
this.analyzer = new SnykCodeAnalyzer(logger, languages, analytics, errorHandler, this.uriAdapter, this.config);
this.analyzer = new SnykCodeAnalyzer(
logger,
languages,
workspace,
analytics,
errorHandler,
this.uriAdapter,
this.config,
);
this.registerAnalyzerProviders(this.analyzer);

this.falsePositiveProvider = new FalsePositiveWebviewProvider(
Expand All @@ -108,6 +116,7 @@ export class SnykCodeService extends AnalysisStatusProvider implements ISnykCode
extensionContext,
this.logger,
languages,
workspace,
codeSettings,
this.learnService,
);
Expand Down
23 changes: 2 additions & 21 deletions src/snyk/snykCode/falsePositive/falsePositive.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Marker } from '@snyk/code-client';
import path from 'path';
import { IVSCodeWorkspace } from '../../common/vscode/workspace';
import { completeFileSuggestionType } from '../interfaces';
import { getAbsoluteMarkerFilePath } from '../utils/analysisUtils';
import { IssueUtils } from '../utils/issueUtils';

export class FalsePositive {
Expand Down Expand Up @@ -35,31 +35,12 @@ export class FalsePositive {
private getFiles(markers: Marker[], uri: string): Set<string> {
const markerPositions = markers.flatMap(marker => marker.pos);
const filesArray = markerPositions.map(markerPosition =>
FalsePositive.getAbsoluteMarkerFilePath(this.workspace, markerPosition.file, uri),
getAbsoluteMarkerFilePath(this.workspace, markerPosition.file, uri),
);

return new Set(filesArray);
}

static getAbsoluteMarkerFilePath(
workspace: IVSCodeWorkspace,
markerFilePath: string,
suggestionFilePath: string,
): string {
if (!markerFilePath) {
// If no filePath reported, use suggestion file path as marker's path. Suggestion path is always absolute.
return suggestionFilePath;
}

const workspaceFolders = workspace.getWorkspaceFolders();
if (workspaceFolders.length > 1) {
return markerFilePath;
}

// The Snyk Code analysis reported marker path is relative when in workspace with a single folder, thus need to convert to an absolute
return path.resolve(workspaceFolders[0], markerFilePath);
}

/**
* May throw an error if file cannot be resolved.
*/
Expand Down
28 changes: 26 additions & 2 deletions src/snyk/snykCode/utils/analysisUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/* eslint-disable @typescript-eslint/no-unsafe-call */
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
import { AnalysisResultLegacy, FilePath, FileSuggestion, Marker, Suggestion } from '@snyk/code-client';
import path from 'path';
import { IVSCodeLanguages } from '../../common/vscode/languages';
import {
DecorationOptions,
Expand All @@ -13,6 +14,7 @@ import {
Uri,
} from '../../common/vscode/types';
import { IUriAdapter } from '../../common/vscode/uri';
import { IVSCodeWorkspace } from '../../common/vscode/workspace';
import {
FILE_IGNORE_ISSUE_BASE_COMMENT_TEXT,
IGNORE_ISSUE_BASE_COMMENT_TEXT,
Expand Down Expand Up @@ -179,16 +181,19 @@ export const createIssuesMarkersDecorationOptions = (

export const createIssueRelatedInformation = (
markersList: Marker[],
fileUri: Uri,
fileUriPath: string,
message: string,
languages: IVSCodeLanguages,
workspace: IVSCodeWorkspace,
uriAdapter: IUriAdapter,
): DiagnosticRelatedInformation[] => {
return markersList.reduce((res, marker) => {
const { msg: markerMsgIdxs, pos: positions } = marker;

positions.forEach(position => {
const positionUri = getAbsoluteMarkerFilePath(workspace, position.file, fileUriPath);
const relatedInfo = languages.createDiagnosticRelatedInformation(
fileUri,
uriAdapter.file(positionUri),
createIssueCorrectRange(position, languages),
createIssueMarkerMsg(message, markerMsgIdxs),
);
Expand Down Expand Up @@ -286,3 +291,22 @@ export const ignoreIssueCommentText = (issueId: string, isFileIgnore?: boolean):
export const isSecurityTypeSuggestion = (suggestion: Suggestion): boolean => {
return suggestion.categories.includes('Security');
};

export const getAbsoluteMarkerFilePath = (
workspace: IVSCodeWorkspace,
markerFilePath: string,
suggestionFilePath: string,
): string => {
if (!markerFilePath) {
// If no filePath reported, use suggestion file path as marker's path. Suggestion path is always absolute.
return suggestionFilePath;
}

const workspaceFolders = workspace.getWorkspaceFolders();
if (workspaceFolders.length > 1) {
return markerFilePath;
}

// The Snyk Code analysis reported marker path is relative when in workspace with a single folder, thus need to convert to an absolute
return path.resolve(workspaceFolders[0], markerFilePath);
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ import { IWebViewProvider, WebviewProvider } from '../../../common/views/webview
import { ExtensionContext } from '../../../common/vscode/extensionContext';
import { IVSCodeLanguages } from '../../../common/vscode/languages';
import { IVSCodeWindow } from '../../../common/vscode/window';
import { IVSCodeWorkspace } from '../../../common/vscode/workspace';
import { ICodeSettings } from '../../codeSettings';
import { WEBVIEW_PANEL_QUALITY_TITLE, WEBVIEW_PANEL_SECURITY_TITLE } from '../../constants/analysis';
import { completeFileSuggestionType, ISnykCodeAnalyzer } from '../../interfaces';
import { messages as errorMessages } from '../../messages/error';
import { createIssueCorrectRange, getVSCodeSeverity } from '../../utils/analysisUtils';
import { createIssueCorrectRange, getAbsoluteMarkerFilePath, getVSCodeSeverity } from '../../utils/analysisUtils';
import { FalsePositiveWebviewModel } from '../falsePositive/falsePositiveWebviewProvider';
import { ICodeSuggestionWebviewProvider } from '../interfaces';

Expand All @@ -42,6 +43,7 @@ export class CodeSuggestionWebviewProvider
protected readonly context: ExtensionContext,
protected readonly logger: ILog,
private readonly languages: IVSCodeLanguages,
private readonly workspace: IVSCodeWorkspace,
private readonly codeSettings: ICodeSettings,
private readonly learnService: LearnService,
) {
Expand Down Expand Up @@ -140,10 +142,16 @@ export class CodeSuggestionWebviewProvider
const { type, args } = message;
switch (type) {
case 'openLocal': {
let { uri, cols, rows } = args;
uri = vscode.Uri.parse(uri as string);
const { uri, cols, rows, suggestionUri } = args as {
uri: string;
cols: [number, number];
rows: [number, number];
suggestionUri: string;
};
const localUriPath = getAbsoluteMarkerFilePath(this.workspace, uri, suggestionUri);
const localUri = vscode.Uri.parse(localUriPath);
const range = createIssueCorrectRange({ cols, rows }, this.languages);
await vscode.commands.executeCommand(SNYK_OPEN_LOCAL_COMMAND, uri, range);
await vscode.commands.executeCommand(SNYK_OPEN_LOCAL_COMMAND, localUri, range);
break;
}
case 'openBrowser': {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,12 @@
},
});
}
function getSuggestionPosition(range?: { rows: any; cols: any }) {
function getSuggestionPosition(position?: { file: string; rows: any; cols: any }) {
return {
uri: suggestion.uri,
rows: range ? range.rows : suggestion.rows,
cols: range ? range.cols : suggestion.cols,
uri: position?.file ?? suggestion.uri,
rows: position ? position.rows : suggestion.rows,
cols: position ? position.cols : suggestion.cols,
suggestionUri: suggestion.uri,
};
}
function previousExample() {
Expand Down
54 changes: 54 additions & 0 deletions src/test/unit/mocks/languages.mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import sinon from 'sinon';
import { IVSCodeLanguages } from '../../../snyk/common/vscode/languages';
import { DiagnosticRelatedInformation, Position, Range as vsRange, Uri } from '../../../snyk/common/vscode/types';

export const languagesMock = {
createDiagnostic: sinon.fake(),
createDiagnosticCollection: sinon.fake(),
registerCodeActionsProvider: sinon.fake(),
registerHoverProvider: sinon.fake(),

createDiagnosticRelatedInformation(
uri: Uri,
rangeOrPosition: vsRange | Position,
message: string,
): DiagnosticRelatedInformation {
const range = isRange(rangeOrPosition)
? rangeOrPosition
: ({
start: {
line: rangeOrPosition.line,
character: rangeOrPosition.character,
},
end: {
line: rangeOrPosition.line,
character: rangeOrPosition.character,
},
} as unknown as vsRange);

return {
message,
location: {
uri,
range,
},
};
},

createRange(startLine: number, startCharacter: number, endLine: number, endCharacter: number): vsRange {
return {
start: {
line: startLine,
character: startCharacter,
} as unknown as Position,
end: {
line: endLine,
character: endCharacter,
} as unknown as Position,
} as unknown as vsRange;
},
} as IVSCodeLanguages;

function isRange(pet: vsRange | Position): pet is vsRange {
return (pet as vsRange).start !== undefined;
}
18 changes: 18 additions & 0 deletions src/test/unit/mocks/uri.mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Uri } from '../../../snyk/common/vscode/types';
import { IUriAdapter } from '../../../snyk/common/vscode/uri';

export class UriAdapterMock implements IUriAdapter {
file(path: string): Uri {
return {
path: path,
} as Uri;
}

parse(path: string): Uri {
return {
path: path,
} as Uri;
}
}

export const uriAdapterMock = new UriAdapterMock();
10 changes: 10 additions & 0 deletions src/test/unit/mocks/workspace.mock.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import * as os from 'os';
import path from 'path';
import { IVSCodeWorkspace } from '../../../snyk/common/vscode/workspace';

export function stubWorkspaceConfiguration<T>(configSetting: string, returnValue: T | undefined): IVSCodeWorkspace {
Expand All @@ -8,3 +10,11 @@ export function stubWorkspaceConfiguration<T>(configSetting: string, returnValue
},
} as IVSCodeWorkspace;
}

export const workspaceMock = {
getWorkspaceFolders() {
return [workspaceFolder];
},
} as IVSCodeWorkspace;

export const workspaceFolder = path.join(os.homedir(), 'snyk/project');
47 changes: 0 additions & 47 deletions src/test/unit/snykCode/falsePositive/falsePositive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,53 +23,6 @@ suite('False Positive', () => {
throws(() => new FalsePositive(workspace, {} as completeFileSuggestionType));
});

test('Returns correct absolute path if no marker file path provided', () => {
// arrange
const suggestionFilePath = '/Users/snyk/goof/test.js';
const markerFilePath = '';

// act
const absoluteFilePath = FalsePositive.getAbsoluteMarkerFilePath(workspace, markerFilePath, suggestionFilePath);

// assert
strictEqual(absoluteFilePath, suggestionFilePath);
});

test('Returns correct absolute path if in multi-folder workspace', () => {
// arrange
const suggestionFilePath = '/Users/snyk/goof/test.js';
const markerFilePath = '/Users/snyk/goof/test2.js';
workspace = {
getWorkspaceFolders: () => ['/Users/snyk/goof1', '/Users/snyk/goof2'],
} as unknown as IVSCodeWorkspace;

// act
const absoluteFilePath = FalsePositive.getAbsoluteMarkerFilePath(workspace, markerFilePath, suggestionFilePath);

// assert
strictEqual(absoluteFilePath, markerFilePath);
});

test('Returns correct absolute path if in single-folder workspace', () => {
// arrange
const suggestionFilePath = '/Users/snyk/goof/test.js';
const relativeMarkerFilePath = 'test2.js';
const workspaceFolder = '/Users/snyk/goof1';
workspace = {
getWorkspaceFolders: () => [workspaceFolder],
} as unknown as IVSCodeWorkspace;

// act
const absoluteFilePath = FalsePositive.getAbsoluteMarkerFilePath(
workspace,
relativeMarkerFilePath,
suggestionFilePath,
);

// assert
strictEqual(absoluteFilePath, path.resolve(workspaceFolder, relativeMarkerFilePath));
});

test('Returns correct generated content', async () => {
// arrange
const filePath = os.platform() === 'win32' ? 'C:\\Users\\snyk\\goof\\test.js' : '/Users/snyk/goof/test.js';
Expand Down
Loading

0 comments on commit 2449c68

Please sign in to comment.