From 80ab044a1e9fae00bcb866d49495d8c0c67a6d2a Mon Sep 17 00:00:00 2001 From: Jeremy Date: Wed, 14 Aug 2019 17:26:50 +0200 Subject: [PATCH] SONAR-12334 Add primary location in snippets --- .../ComponentSourceSnippetViewer.tsx | 7 +- .../ComponentSourceSnippetViewer-test.tsx | 29 ++++++- .../__tests__/utils-test.ts | 22 ++++- .../crossComponentSourceViewer/utils.ts | 84 +++++++++++-------- 4 files changed, 104 insertions(+), 38 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx index d9e14742a19..ef71fd3bc5f 100644 --- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx +++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx @@ -21,10 +21,10 @@ import * as React from 'react'; import * as classNames from 'classnames'; import { createSnippets, expandSnippet, EXPAND_BY_LINES, MERGE_DISTANCE } from './utils'; import SnippetViewer from './SnippetViewer'; -import SourceViewerHeaderSlim from '../../../components/SourceViewer/SourceViewerHeaderSlim'; -import getCoverageStatus from '../../../components/SourceViewer/helpers/getCoverageStatus'; import { getSources } from '../../../api/components'; +import getCoverageStatus from '../../../components/SourceViewer/helpers/getCoverageStatus'; import { locationsByLine } from '../../../components/SourceViewer/helpers/indexing'; +import SourceViewerHeaderSlim from '../../../components/SourceViewer/SourceViewerHeaderSlim'; import { getBranchLikeQuery } from '../../../helpers/branches'; interface Props { @@ -83,7 +83,8 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent 0 ? this.props.issue : undefined ); this.setState({ snippets }); } diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx index a1e7fdd29ff..d988ce2e7ce 100644 --- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx +++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx @@ -19,7 +19,7 @@ */ import * as React from 'react'; import { shallow } from 'enzyme'; -import { times } from 'lodash'; +import { range, times } from 'lodash'; import ComponentSourceSnippetViewer from '../ComponentSourceSnippetViewer'; import { mockMainBranch, @@ -45,6 +45,33 @@ it('should render correctly', () => { expect(shallowRender()).toMatchSnapshot(); }); +it('should render correctly with secondary locations', () => { + // issue with secondary locations but no flows + const issue = mockIssue(true, { + flows: [], + textRange: { startLine: 5, endLine: 5, startOffset: 5, endOffset: 10 } + }); + + const snippetGroup: T.SnippetGroup = { + locations: [ + mockFlowLocation({ + component: 'a', + textRange: { startLine: 34, endLine: 34, startOffset: 0, endOffset: 0 } + }), + mockFlowLocation({ + component: 'a', + textRange: { startLine: 54, endLine: 54, startOffset: 0, endOffset: 0 } + }) + ], + ...mockSnippetsByComponent('a', [...range(3, 15), 32, 33, 34, 35, 36, 52, 53, 54, 55, 56]) + }; + const wrapper = shallowRender({ issue, snippetGroup }); + expect(wrapper.state('snippets')).toHaveLength(3); + expect(wrapper.state('snippets')[0]).toHaveLength(12); + expect(wrapper.state('snippets')[1]).toHaveLength(5); + expect(wrapper.state('snippets')[2]).toHaveLength(5); +}); + it('should expand block', async () => { (getSources as jest.Mock).mockResolvedValueOnce( Object.values(mockSnippetsByComponent('a', [22, 23, 24, 25, 26, 27, 28, 29, 30, 31]).sources) diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts index ad114d760a3..667136fd3df 100644 --- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts +++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts @@ -18,9 +18,10 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import { keyBy, range } from 'lodash'; -import { groupLocationsByComponent, createSnippets, expandSnippet } from '../utils'; +import { createSnippets, expandSnippet, groupLocationsByComponent } from '../utils'; import { mockFlowLocation, + mockIssue, mockSnippetsByComponent, mockSourceLine } from '../../../../helpers/testMocks'; @@ -147,6 +148,25 @@ describe('createSnippets', () => { expect(results[0]).toHaveLength(11); expect(results[1]).toHaveLength(5); }); + + it("should prepend the issue's main location if necessary", () => { + const results = createSnippets( + [ + mockFlowLocation({ + textRange: { startLine: 47, startOffset: 2, endLine: 47, endOffset: 3 } + }), + mockFlowLocation({ + textRange: { startLine: 22, startOffset: 2, endLine: 22, endOffset: 3 } + }) + ], + mockSnippetsByComponent('', range(3, 15).concat(range(20, 25), range(45, 50))).sources, + false, + mockIssue(false, { textRange: { startLine: 5, endLine: 5, startOffset: 0, endOffset: 0 } }) + ); + + expect(results).toHaveLength(3); + expect(results[0]).toHaveLength(12); + }); }); describe('expandSnippet', () => { diff --git a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/utils.ts b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/utils.ts index 5c34f42dfbb..1e3bf967ed4 100644 --- a/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/utils.ts +++ b/server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/utils.ts @@ -42,50 +42,68 @@ function collision([startA, endA]: number[], [startB, endB]: number[]) { return !(startA > endB + MERGE_DISTANCE || endA < startB - MERGE_DISTANCE); } +function getPrimaryLocation(issue: T.Issue): T.FlowLocation { + return { + component: issue.component, + textRange: issue.textRange || { + endLine: 0, + endOffset: 0, + startLine: 0, + startOffset: 0 + } + }; +} + export function createSnippets( locations: T.FlowLocation[], componentLines: T.LineMap = {}, - last: boolean + last: boolean, + issue?: T.Issue ): T.SourceLine[][] { return rangesToSnippets( // For each location's range (2 above and 2 below), and then compare with other ranges // to merge snippets that collide. - locations.reduce((snippets: Array<{ start: number; end: number }>, loc, index) => { - const startIndex = Math.max(1, loc.textRange.startLine - LINES_ABOVE); - const endIndex = - loc.textRange.endLine + - (last && index === locations.length - 1 ? LINES_BELOW_LAST : LINES_BELOW); - - let firstCollision: { start: number; end: number } | undefined; - - // Remove ranges that collide into the first collision - snippets = snippets.filter(snippet => { - if (collision([snippet.start, snippet.end], [startIndex, endIndex])) { - let keep = false; - // Check if we've already collided - if (!firstCollision) { - firstCollision = snippet; - keep = true; + (issue ? [getPrimaryLocation(issue), ...locations] : locations).reduce( + (snippets: Array<{ start: number; end: number }>, loc, index) => { + const startIndex = Math.max(1, loc.textRange.startLine - LINES_ABOVE); + const endIndex = + loc.textRange.endLine + + ((issue && index === 0) || (last && index === locations.length - 1) + ? LINES_BELOW_LAST + : LINES_BELOW); + + let firstCollision: { start: number; end: number } | undefined; + + // Remove ranges that collide into the first collision + snippets = snippets.filter(snippet => { + if (collision([snippet.start, snippet.end], [startIndex, endIndex])) { + let keep = false; + // Check if we've already collided + if (!firstCollision) { + firstCollision = snippet; + keep = true; + } + // Merge with first collision: + firstCollision.start = Math.min(startIndex, snippet.start, firstCollision.start); + firstCollision.end = Math.max(endIndex, snippet.end, firstCollision.end); + + // remove the range if it was not the first collision + return keep; } - // Merge with first collision: - firstCollision.start = Math.min(startIndex, snippet.start, firstCollision.start); - firstCollision.end = Math.max(endIndex, snippet.end, firstCollision.end); + return true; + }); - // remove the range if it was not the first collision - return keep; + if (firstCollision === undefined) { + snippets.push({ + start: startIndex, + end: endIndex + }); } - return true; - }); - - if (firstCollision === undefined) { - snippets.push({ - start: startIndex, - end: endIndex - }); - } - return snippets; - }, []), + return snippets; + }, + [] + ), componentLines ); } -- 2.39.5