From: Stas Vilchik Date: Wed, 21 Mar 2018 13:18:13 +0000 (+0100) Subject: SONAR-10489 Support cross file issue locations in web app X-Git-Tag: 7.5~1472 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=16ae386379cdfd39ff29ae9e391ddea115fea1ef;p=sonarqube.git SONAR-10489 Support cross file issue locations in web app --- diff --git a/server/sonar-web/src/main/js/app/types.ts b/server/sonar-web/src/main/js/app/types.ts index 922cbf8cf69..8155161aca0 100644 --- a/server/sonar-web/src/main/js/app/types.ts +++ b/server/sonar-web/src/main/js/app/types.ts @@ -150,6 +150,8 @@ export interface FacetValue { } export interface FlowLocation { + component: string; + componentName?: string; msg: string; textRange: TextRange; } diff --git a/server/sonar-web/src/main/js/apps/issues/components/App.tsx b/server/sonar-web/src/main/js/apps/issues/components/App.tsx index 8eb5c82a6a9..f6ba42afa59 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/App.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/App.tsx @@ -347,7 +347,14 @@ export default class App extends React.PureComponent { }; if (this.state.openIssue) { if (path.query.open && path.query.open === this.state.openIssue.key) { - this.scrollToSelectedIssue(); + this.setState( + { + locationsNavigator: false, + selectedFlowIndex: undefined, + selectedLocationIndex: undefined + }, + this.scrollToSelectedIssue + ); } else { this.context.router.replace(path); } @@ -384,7 +391,7 @@ export default class App extends React.PureComponent { if (selected) { const element = document.querySelector(`[data-issue="${selected}"]`); if (element) { - scrollToElement(element, { topOffset: 150, bottomOffset: 100, smooth }); + scrollToElement(element, { topOffset: 250, bottomOffset: 100, smooth }); } } }; @@ -993,6 +1000,8 @@ export default class App extends React.PureComponent { component={component} issue={openIssue} organization={this.props.organization} + selectedFlowIndex={this.state.selectedFlowIndex} + selectedLocationIndex={this.state.selectedLocationIndex} /> ) : ( @@ -1020,14 +1029,13 @@ export default class App extends React.PureComponent { ) : ( this.renderList() diff --git a/server/sonar-web/src/main/js/apps/issues/components/ComponentBreadcrumbs.tsx b/server/sonar-web/src/main/js/apps/issues/components/ComponentBreadcrumbs.tsx index ffeaae34231..b5f7a6fe21a 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/ComponentBreadcrumbs.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/ComponentBreadcrumbs.tsx @@ -19,7 +19,8 @@ */ import * as React from 'react'; import { Link } from 'react-router'; -import { BranchLike, Component } from '../../../app/types'; +import { getSelectedLocation } from '../utils'; +import { BranchLike, Component, Issue } from '../../../app/types'; import Organization from '../../../components/shared/Organization'; import { collapsePath, limitComponentName } from '../../../helpers/path'; import { getBranchLikeUrl, getCodeUrl } from '../../../helpers/urls'; @@ -27,29 +28,40 @@ import { getBranchLikeUrl, getCodeUrl } from '../../../helpers/urls'; interface Props { branchLike?: BranchLike; component?: Component; - issue: { - component: string; - componentLongName: string; - organization: string; - project: string; - projectName: string; - subProject?: string; - subProjectName?: string; - }; + issue: Pick< + Issue, + | 'component' + | 'componentLongName' + | 'flows' + | 'organization' + | 'project' + | 'projectName' + | 'secondaryLocations' + | 'subProject' + | 'subProjectName' + >; organization: { key: string } | undefined; + selectedFlowIndex?: number; + selectedLocationIndex?: number; } export default function ComponentBreadcrumbs({ branchLike, component, issue, - organization + organization, + selectedFlowIndex, + selectedLocationIndex }: Props) { const displayOrganization = !organization && (!component || ['VW', 'SVW'].includes(component.qualifier)); const displayProject = !component || !['TRK', 'BRC', 'DIR'].includes(component.qualifier); const displaySubProject = !component || !['BRC', 'DIR'].includes(component.qualifier); + const selectedLocation = getSelectedLocation(issue, selectedFlowIndex, selectedLocationIndex); + const componentKey = selectedLocation ? selectedLocation.component : issue.component; + const componentName = selectedLocation ? selectedLocation.componentName : issue.componentLongName; + return (
{displayOrganization && ( @@ -76,10 +88,8 @@ export default function ComponentBreadcrumbs({ )} - - {collapsePath(issue.componentLongName)} + + {collapsePath(componentName || '')}
); diff --git a/server/sonar-web/src/main/js/apps/issues/components/IssuesSourceViewer.tsx b/server/sonar-web/src/main/js/apps/issues/components/IssuesSourceViewer.tsx index 930976c7cac..6266c18322c 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/IssuesSourceViewer.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/IssuesSourceViewer.tsx @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import * as React from 'react'; +import { getLocations, getSelectedLocation } from '../utils'; import { BranchLike, Issue } from '../../../app/types'; import SourceViewer from '../../../components/SourceViewer/SourceViewer'; import { scrollToElement } from '../../../helpers/scrolling'; @@ -25,6 +26,7 @@ import { scrollToElement } from '../../../helpers/scrolling'; interface Props { branchLike: BranchLike | undefined; loadIssues: (component: string, from: number, to: number) => Promise; + locationsNavigator: boolean; onIssueChange: (issue: Issue) => void; onIssueSelect: (issueKey: string) => void; onLocationSelect: (index: number) => void; @@ -71,53 +73,57 @@ export default class IssuesSourceViewer extends React.PureComponent { render() { const { openIssue, selectedFlowIndex, selectedLocationIndex } = this.props; - const locations = - selectedFlowIndex !== undefined - ? openIssue.flows[selectedFlowIndex] - : openIssue.flows.length > 0 ? openIssue.flows[0] : openIssue.secondaryLocations; - - let locationMessage = undefined; - let locationLine = undefined; + const locations = getLocations(openIssue, selectedFlowIndex); + const selectedLocation = getSelectedLocation( + openIssue, + selectedFlowIndex, + selectedLocationIndex + ); - // We don't want to display a location message when selected location is -1 - if ( - locations !== undefined && - selectedLocationIndex !== undefined && - selectedLocationIndex >= 0 && - locations.length >= selectedLocationIndex - ) { - locationMessage = { - index: selectedLocationIndex, - text: locations[selectedLocationIndex].msg - }; - locationLine = locations[selectedLocationIndex].textRange.startLine; - } + const component = selectedLocation ? selectedLocation.component : openIssue.component; // if location is selected, show (and load) code around it // otherwise show code around the open issue - const aroundLine = locationLine || (openIssue.textRange && openIssue.textRange.endLine); + const aroundLine = selectedLocation + ? selectedLocation.textRange.startLine + : openIssue.textRange && openIssue.textRange.endLine; + + // replace locations in another file with `undefined` to keep the same location indexes + const highlightedLocations = locations.map( + location => (location.component === component ? location : undefined) + ); + + const highlightedLocationMessage = + this.props.locationsNavigator && selectedLocationIndex !== undefined + ? selectedLocation && { index: selectedLocationIndex, text: selectedLocation.msg } + : undefined; const allMessagesEmpty = locations !== undefined && locations.every(location => !location.msg); + // do not load issues when open another file for a location + const loadIssues = + component === openIssue.component ? this.props.loadIssues : () => Promise.resolve([]); + const selectedIssue = component === openIssue.component ? openIssue.key : undefined; + return (
(this.node = node)}>
); diff --git a/server/sonar-web/src/main/js/apps/issues/components/__tests__/ComponentBreadcrumbs-test.tsx b/server/sonar-web/src/main/js/apps/issues/components/__tests__/ComponentBreadcrumbs-test.tsx index a6e4e7971ba..0e6fd7f9652 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/__tests__/ComponentBreadcrumbs-test.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/__tests__/ComponentBreadcrumbs-test.tsx @@ -25,9 +25,11 @@ import { ShortLivingBranch, BranchType } from '../../../../app/types'; const baseIssue = { component: 'comp', componentLongName: 'comp-name', + flows: [], organization: 'org', project: 'proj', - projectName: 'proj-name' + projectName: 'proj-name', + secondaryLocations: [] }; it('renders', () => { diff --git a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocations.tsx b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocations.tsx index 12782ff6b57..3a2cdb9885f 100644 --- a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocations.tsx +++ b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocations.tsx @@ -22,7 +22,7 @@ import ConciseIssueLocationBadge from './ConciseIssueLocationBadge'; import { Issue } from '../../../app/types'; interface Props { - issue: Issue; + issue: Pick; onFlowSelect: (index: number) => void; selectedFlowIndex: number | undefined; } diff --git a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocationsNavigator.tsx b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocationsNavigator.tsx index d6ff0d7b32b..60946f692f5 100644 --- a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocationsNavigator.tsx +++ b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocationsNavigator.tsx @@ -18,11 +18,14 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import * as React from 'react'; +import { uniq } from 'lodash'; import ConciseIssueLocationsNavigatorLocation from './ConciseIssueLocationsNavigatorLocation'; +import CrossFileLocationsNavigator from './CrossFileLocationsNavigator'; +import { getLocations } from '../utils'; import { Issue } from '../../../app/types'; interface Props { - issue: Issue; + issue: Pick; onLocationSelect: (index: number) => void; scroll: (element: Element) => void; selectedFlowIndex: number | undefined; @@ -31,31 +34,43 @@ interface Props { export default class ConciseIssueLocationsNavigator extends React.PureComponent { render() { - const { selectedFlowIndex, selectedLocationIndex } = this.props; - const { flows, secondaryLocations } = this.props.issue; - - const locations = - selectedFlowIndex !== undefined - ? flows[selectedFlowIndex] - : flows.length > 0 ? flows[0] : secondaryLocations; + const locations = getLocations(this.props.issue, this.props.selectedFlowIndex); if (!locations || locations.length === 0 || locations.every(location => !location.msg)) { return null; } - return ( -
- {locations.map((location, index) => ( - - ))} -
- ); + const locationComponents = [ + this.props.issue.component, + ...locations.map(location => location.component) + ]; + const isCrossFile = uniq(locationComponents).length > 1; + + if (isCrossFile) { + return ( + + ); + } else { + return ( +
+ {locations.map((location, index) => ( + + ))} +
+ ); + } } } diff --git a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocationsNavigatorLocation.tsx b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocationsNavigatorLocation.tsx index 455b597b9ee..1fe90aa029e 100644 --- a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocationsNavigatorLocation.tsx +++ b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/ConciseIssueLocationsNavigatorLocation.tsx @@ -53,7 +53,7 @@ export default class ConciseIssueLocationsNavigatorLocation extends React.PureCo return (
(this.node = node)}> {this.props.index + 1} diff --git a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/CrossFileLocationsNavigator.tsx b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/CrossFileLocationsNavigator.tsx new file mode 100644 index 00000000000..a2826a6b47b --- /dev/null +++ b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/CrossFileLocationsNavigator.tsx @@ -0,0 +1,190 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import * as React from 'react'; +import ConciseIssueLocationsNavigatorLocation from './ConciseIssueLocationsNavigatorLocation'; +import { Issue, FlowLocation } from '../../../app/types'; +import { translateWithParameters } from '../../../helpers/l10n'; +import { collapsePath } from '../../../helpers/path'; + +interface Props { + issue: Pick; + locations: FlowLocation[]; + onLocationSelect: (index: number) => void; + scroll: (element: Element) => void; + selectedLocationIndex: number | undefined; +} + +interface State { + collapsed: boolean; +} + +interface LocationGroup { + component: string | undefined; + componentName: string | undefined; + firstLocationIndex: number; + locations: FlowLocation[]; +} + +export default class CrossFileLocationsNavigator extends React.PureComponent { + state: State = { collapsed: true }; + + componentWillReceiveProps(nextProps: Props) { + if (nextProps.issue.key !== this.props.issue.key) { + this.setState({ collapsed: true }); + } + + // expand locations list as soon as a location in the middle of the list is selected + const { locations: nextLocations } = nextProps; + if ( + nextProps.selectedLocationIndex && + nextProps.selectedLocationIndex > 0 && + nextLocations !== undefined && + nextProps.selectedLocationIndex < nextLocations.length - 1 + ) { + this.setState({ collapsed: false }); + } + } + + handleMoreLocationsClick = (event: React.MouseEvent) => { + event.preventDefault(); + event.currentTarget.blur(); + this.setState({ collapsed: false }); + }; + + groupByFile = (locations: FlowLocation[]) => { + const groups: LocationGroup[] = []; + + let currentLocations: FlowLocation[] = []; + let currentComponent: string | undefined; + let currentComponentName: string | undefined; + let currentFirstLocationIndex = 0; + + for (let index = 0; index < locations.length; index++) { + const location = locations[index]; + if (location.component === currentComponent) { + currentLocations.push(location); + } else { + if (currentLocations.length > 0) { + groups.push({ + component: currentComponent, + componentName: currentComponentName, + firstLocationIndex: currentFirstLocationIndex, + locations: currentLocations + }); + } + currentLocations = [location]; + currentComponent = location.component; + currentComponentName = location.componentName; + currentFirstLocationIndex = index; + } + } + + if (currentLocations.length > 0) { + groups.push({ + component: currentComponent, + componentName: currentComponentName, + firstLocationIndex: currentFirstLocationIndex, + locations: currentLocations + }); + } + + return groups; + }; + + renderLocation = (index: number, message: string) => { + return ( + + ); + }; + + renderGroup = ( + group: LocationGroup, + groupIndex: number, + { onlyFirst = false, onlyLast = false } = {} + ) => { + const { firstLocationIndex } = group; + const lastLocationIndex = group.locations.length - 1; + return ( +
+
+ + {collapsePath(group.componentName || '', 15)} +
+ {group.locations.length > 0 && ( +
+ {onlyFirst && this.renderLocation(firstLocationIndex, group.locations[0].msg)} + + {onlyLast && + this.renderLocation( + firstLocationIndex + lastLocationIndex, + group.locations[lastLocationIndex].msg + )} + + {!onlyFirst && + !onlyLast && + group.locations.map((location, index) => + this.renderLocation(firstLocationIndex + index, location.msg) + )} +
+ )} +
+ ); + }; + + render() { + const { locations } = this.props; + const groups = this.groupByFile(locations); + + if (locations.length > 2 && groups.length > 1 && this.state.collapsed) { + const firstGroup = groups[0]; + const lastGroup = groups[groups.length - 1]; + return ( + + ); + } else { + return ( +
+ {groups.map((group, groupIndex) => this.renderGroup(group, groupIndex))} +
+ ); + } + } +} diff --git a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/ConciseIssueLocations-test.tsx b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/ConciseIssueLocations-test.tsx index d9aee370385..ad7d541f740 100644 --- a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/ConciseIssueLocations-test.tsx +++ b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/ConciseIssueLocations-test.tsx @@ -23,32 +23,12 @@ import ConciseIssueLocations from '../ConciseIssueLocations'; const textRange = { startLine: 1, startOffset: 1, endLine: 1, endOffset: 1 }; -const baseIssue = { - component: '', - componentLongName: '', - componentQualifier: '', - componentUuid: '', - creationDate: '', - key: '', - message: '', - organization: '', - project: '', - projectName: '', - projectOrganization: '', - projectUuid: '', - rule: '', - ruleName: '', - severity: '', - status: '', - type: '', - secondaryLocations: [], - flows: [] -}; +const loc = { component: '', msg: '', textRange }; it('should render secondary locations', () => { const issue = { - ...baseIssue, - secondaryLocations: [{ msg: '', textRange }, { msg: '', textRange }, { msg: '', textRange }] + flows: [], + secondaryLocations: [loc, loc, loc] }; expect( shallow( @@ -59,9 +39,8 @@ it('should render secondary locations', () => { it('should render one flow', () => { const issue = { - ...baseIssue, - secondaryLocations: [], - flows: [[{ msg: '', textRange }, { msg: '', textRange }, { msg: '', textRange }]] + flows: [[loc, loc, loc]], + secondaryLocations: [] }; expect( shallow( @@ -72,12 +51,8 @@ it('should render one flow', () => { it('should render several flows', () => { const issue = { - ...baseIssue, - flows: [ - [{ msg: '', textRange }, { msg: '', textRange }, { msg: '', textRange }], - [{ msg: '', textRange }, { msg: '', textRange }], - [{ msg: '', textRange }, { msg: '', textRange }, { msg: '', textRange }] - ] + flows: [[loc, loc, loc], [loc, loc], [loc, loc, loc]], + secondaryLocations: [] }; expect( shallow( diff --git a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/ConciseIssueLocationsNavigator-test.tsx b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/ConciseIssueLocationsNavigator-test.tsx new file mode 100644 index 00000000000..9ae00c5b9c4 --- /dev/null +++ b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/ConciseIssueLocationsNavigator-test.tsx @@ -0,0 +1,138 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import * as React from 'react'; +import { shallow } from 'enzyme'; +import ConciseIssueLocationsNavigator from '../ConciseIssueLocationsNavigator'; +import { FlowLocation } from '../../../../app/types'; + +const location1: FlowLocation = { + component: 'foo', + componentName: 'src/foo.js', + msg: 'Do not use foo', + textRange: { startLine: 7, endLine: 7, startOffset: 5, endOffset: 8 } +}; + +const location2: FlowLocation = { + component: 'foo', + componentName: 'src/foo.js', + msg: 'Do not use foo', + textRange: { startLine: 8, endLine: 8, startOffset: 0, endOffset: 5 } +}; + +const location3: FlowLocation = { + component: 'bar', + componentName: 'src/bar.js', + msg: 'Do not use bar', + textRange: { startLine: 15, endLine: 16, startOffset: 4, endOffset: 6 } +}; + +it('should render secondary locations in the same file', () => { + const issue = { + component: 'foo', + key: '', + flows: [], + secondaryLocations: [location1, location2] + }; + expect( + shallow( + + ) + ).toMatchSnapshot(); +}); + +it('should render flow locations in the same file', () => { + const issue = { + component: 'foo', + key: '', + flows: [[location1, location2]], + secondaryLocations: [] + }; + expect( + shallow( + + ) + ).toMatchSnapshot(); +}); + +it('should render selected flow locations in the same file', () => { + const issue = { + component: 'foo', + key: '', + flows: [[location1, location2]], + secondaryLocations: [location1] + }; + expect( + shallow( + + ) + ).toMatchSnapshot(); +}); + +it('should render flow locations in different file', () => { + const issue = { + component: 'foo', + key: '', + flows: [[location1, location3]], + secondaryLocations: [] + }; + expect( + shallow( + + ) + ).toMatchSnapshot(); +}); + +it('should not render locations', () => { + const issue = { component: 'foo', key: '', flows: [], secondaryLocations: [] }; + const wrapper = shallow( + + ); + expect(wrapper.type()).toBeNull(); +}); diff --git a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/CrossFileLocationsNavigator-test.tsx b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/CrossFileLocationsNavigator-test.tsx new file mode 100644 index 00000000000..f09e6d090d6 --- /dev/null +++ b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/CrossFileLocationsNavigator-test.tsx @@ -0,0 +1,108 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import * as React from 'react'; +import { shallow } from 'enzyme'; +import CrossFileLocationsNavigator from '../CrossFileLocationsNavigator'; +import { FlowLocation } from '../../../../app/types'; +import { click } from '../../../../helpers/testUtils'; + +const location1: FlowLocation = { + component: 'foo', + componentName: 'src/foo.js', + msg: 'Do not use foo', + textRange: { startLine: 7, endLine: 7, startOffset: 5, endOffset: 8 } +}; + +const location2: FlowLocation = { + component: 'foo', + componentName: 'src/foo.js', + msg: 'Do not use foo', + textRange: { startLine: 8, endLine: 8, startOffset: 0, endOffset: 5 } +}; + +const location3: FlowLocation = { + component: 'bar', + componentName: 'src/bar.js', + msg: 'Do not use bar', + textRange: { startLine: 15, endLine: 16, startOffset: 4, endOffset: 6 } +}; + +it('should render', () => { + const wrapper = shallow( + + ); + expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('ConciseIssueLocationsNavigatorLocation').length).toBe(2); + + click(wrapper.find('.concise-issue-location-file-more')); + expect(wrapper.find('ConciseIssueLocationsNavigatorLocation').length).toBe(3); +}); + +it('should render all locations', () => { + const wrapper = shallow( + + ); + expect(wrapper.find('ConciseIssueLocationsNavigatorLocation').length).toBe(2); +}); + +it('should expand all locations', () => { + const wrapper = shallow( + + ); + expect(wrapper.find('ConciseIssueLocationsNavigatorLocation').length).toBe(2); + + wrapper.setProps({ selectedLocationIndex: 1 }); + expect(wrapper.find('ConciseIssueLocationsNavigatorLocation').length).toBe(3); +}); + +it('should collapse locations when issue changes', () => { + const wrapper = shallow( + + ); + wrapper.setProps({ selectedLocationIndex: 1 }); + expect(wrapper.find('ConciseIssueLocationsNavigatorLocation').length).toBe(3); + + wrapper.setProps({ issue: { key: 'def' }, selectedLocationIndex: undefined }); + expect(wrapper.find('ConciseIssueLocationsNavigatorLocation').length).toBe(2); +}); diff --git a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/__snapshots__/ConciseIssueLocationsNavigator-test.tsx.snap b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/__snapshots__/ConciseIssueLocationsNavigator-test.tsx.snap new file mode 100644 index 00000000000..f2512176f96 --- /dev/null +++ b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/__snapshots__/ConciseIssueLocationsNavigator-test.tsx.snap @@ -0,0 +1,136 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`should render flow locations in different file 1`] = ` + +`; + +exports[`should render flow locations in the same file 1`] = ` +
+ + +
+`; + +exports[`should render secondary locations in the same file 1`] = ` +
+ + +
+`; + +exports[`should render selected flow locations in the same file 1`] = ` +
+ + +
+`; diff --git a/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/__snapshots__/CrossFileLocationsNavigator-test.tsx.snap b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/__snapshots__/CrossFileLocationsNavigator-test.tsx.snap new file mode 100644 index 00000000000..e1b734ee3e5 --- /dev/null +++ b/server/sonar-web/src/main/js/apps/issues/conciseIssuesList/__tests__/__snapshots__/CrossFileLocationsNavigator-test.tsx.snap @@ -0,0 +1,76 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`should render 1`] = ` +
+
+
+ + src/foo.js +
+
+ +
+
+ +
+
+ + src/bar.js +
+
+ +
+
+
+`; diff --git a/server/sonar-web/src/main/js/apps/issues/styles.css b/server/sonar-web/src/main/js/apps/issues/styles.css index 9a285c2c33e..0add575a878 100644 --- a/server/sonar-web/src/main/js/apps/issues/styles.css +++ b/server/sonar-web/src/main/js/apps/issues/styles.css @@ -123,12 +123,95 @@ margin-bottom: 4px; } -.consice-issue-locations-navigator-location { - display: flex; +.concise-issue-locations-navigator-location { + position: relative; + z-index: var(--aboveNormalZIndex); + display: inline-flex; align-items: flex-start; + max-width: 100%; border: none; } +.concise-issue-locations-navigator-file { + position: relative; +} + +.concise-issue-locations-navigator-file + .concise-issue-locations-navigator-file { + margin-top: calc(1.5 * var(--gridSize)); +} + +.concise-issue-locations-navigator-file:not(:last-child)::before { + position: absolute; + display: block; + width: 0; + top: 13px; + bottom: calc(-2 * var(--gridSize)); + left: 4px; + border-left: 1px dotted #d18582; + content: ''; +} + +.concise-issue-location-file { + height: calc(2 * var(--gridSize)); + padding-bottom: calc(0.5 * var(--gridSize)); + font-size: var(--smallFontSize); + font-weight: bold; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.concise-issue-location-file-circle, +.concise-issue-location-file-circle-multiple, +.concise-issue-location-file-circle-multiple::before, +.concise-issue-location-file-circle-multiple::after { + position: relative; + top: 1px; + display: inline-block; + width: calc(1px + var(--gridSize)); + height: calc(1px + var(--gridSize)); + border: 1px solid #d18582; + border-radius: 100%; + box-sizing: border-box; + background-color: #ffeaea; +} + +.concise-issue-location-file-circle-multiple { + top: -2px; +} + +.concise-issue-location-file-circle-multiple::before { + position: absolute; + z-index: calc(5 + var(--normalZIndex)); + top: 2px; + left: -1px; + content: ''; +} + +.concise-issue-location-file-circle-multiple::after { + position: absolute; + z-index: calc(5 + var(--aboveNormalZIndex)); + top: 5px; + left: -1px; + content: ''; +} + +.concise-issue-location-file-locations { + padding-left: calc(2 * var(--gridSize)); +} + +.concise-issue-location-file-more { + border-color: rgba(209, 133, 130, 0.2); + color: rgb(209, 133, 130) !important; + font-style: italic; + font-weight: normal; +} + +.concise-issue-location-file-more:hover, +.concise-issue-location-file-more:focus { + border-color: rgba(209, 133, 130, 0.6); +} + .issues-my-issues-filter { margin-bottom: 24px; text-align: center; diff --git a/server/sonar-web/src/main/js/apps/issues/utils.ts b/server/sonar-web/src/main/js/apps/issues/utils.ts index a8ca8b1754e..f148547d9e1 100644 --- a/server/sonar-web/src/main/js/apps/issues/utils.ts +++ b/server/sonar-web/src/main/js/apps/issues/utils.ts @@ -19,6 +19,7 @@ */ import { searchMembers } from '../../api/organizations'; import { searchUsers } from '../../api/users'; +import { Issue } from '../../app/types'; import { formatMeasure } from '../../helpers/measures'; import { queriesEqual, @@ -227,3 +228,31 @@ const save = (value: string) => { export const saveMyIssues = (myIssues: boolean) => save(myIssues ? LOCALSTORAGE_MY : LOCALSTORAGE_ALL); + +export function getLocations( + { flows, secondaryLocations }: Pick, + selectedFlowIndex: number | undefined +) { + if (selectedFlowIndex !== undefined) { + return flows[selectedFlowIndex] || []; + } else { + return flows.length > 0 ? flows[0] : secondaryLocations; + } +} + +export function getSelectedLocation( + issue: Pick, + selectedFlowIndex: number | undefined, + selectedLocationIndex: number | undefined +) { + const locations = getLocations(issue, selectedFlowIndex); + if ( + selectedLocationIndex !== undefined && + selectedLocationIndex >= 0 && + locations.length >= selectedLocationIndex + ) { + return locations[selectedLocationIndex]; + } else { + return undefined; + } +} diff --git a/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerBase.tsx b/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerBase.tsx index 101bc211a1a..629b4d86971 100644 --- a/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerBase.tsx +++ b/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerBase.tsx @@ -63,7 +63,9 @@ interface Props { displayIssueLocationsLink?: boolean; displayLocationMarkers?: boolean; highlightedLine?: number; - highlightedLocations?: FlowLocation[]; + // `undefined` elements mean they are located in a different file, + // but kept to maintaint the location indexes + highlightedLocations?: (FlowLocation | undefined)[]; highlightedLocationMessage?: { index: number; text: string }; loadComponent?: ( component: string, @@ -158,6 +160,14 @@ export default class SourceViewerBase extends React.PureComponent } componentWillReceiveProps(nextProps: Props) { + // if a component or a branch has changed, + // set `loading: true` immediately to avoid unwanted scrolling in `LineCode` + if ( + nextProps.component !== this.props.component || + !isSameBranchLike(nextProps.branchLike, this.props.branchLike) + ) { + this.setState({ loading: true }); + } if ( nextProps.onIssueSelect !== undefined && nextProps.selectedIssue !== this.props.selectedIssue diff --git a/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerCode.tsx b/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerCode.tsx index 96bd3512e39..ef1abb5af32 100644 --- a/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerCode.tsx +++ b/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerCode.tsx @@ -54,7 +54,9 @@ interface Props { hasSourcesBefore: boolean; highlightedLine: number | undefined; highlightedLocationMessage: { index: number; text: string } | undefined; - highlightedLocations: FlowLocation[] | undefined; + // `undefined` elements mean they are located in a different file, + // but kept to maintain the location indexes + highlightedLocations: (FlowLocation | undefined)[] | undefined; highlightedSymbols: string[]; issueLocationsByLine: { [line: number]: LinearIssueLocation[] }; issuePopup: { issue: string; name: string } | undefined; @@ -102,9 +104,11 @@ export default class SourceViewerCode extends React.PureComponent { return EMPTY_ARRAY; } return highlightedLocations.reduce((locations, location, index) => { - const linearLocations: LinearIssueLocation[] = getLinearLocations(location.textRange) - .filter(l => l.line === line.line) - .map(l => ({ ...l, startLine: location.textRange.startLine, index })); + const linearLocations: LinearIssueLocation[] = location + ? getLinearLocations(location.textRange) + .filter(l => l.line === line.line) + .map(l => ({ ...l, startLine: location.textRange.startLine, index })) + : []; return [...locations, ...linearLocations]; }, []); }; diff --git a/server/sonar-web/src/main/js/helpers/__tests__/issues-test.ts b/server/sonar-web/src/main/js/helpers/__tests__/issues-test.ts index fa64839dfa4..2db7dff1534 100644 --- a/server/sonar-web/src/main/js/helpers/__tests__/issues-test.ts +++ b/server/sonar-web/src/main/js/helpers/__tests__/issues-test.ts @@ -60,16 +60,61 @@ it('should populate comments data', () => { it('orders secondary locations', () => { const issue = { flows: [ - { locations: [{ textRange: { startLine: 68, startOffset: 5, endLine: 68, endOffset: 7 } }] }, - { locations: [{ textRange: { startLine: 43, startOffset: 8, endLine: 43, endOffset: 12 } }] }, - { locations: [{ textRange: { startLine: 43, startOffset: 6, endLine: 43, endOffset: 8 } }] }, - { locations: [{ textRange: { startLine: 70, startOffset: 12, endLine: 70, endOffset: 16 } }] } + { + locations: [ + { + component: 'foo', + textRange: { startLine: 68, startOffset: 5, endLine: 68, endOffset: 7 } + } + ] + }, + { + locations: [ + { + component: 'unknown', + textRange: { startLine: 43, startOffset: 8, endLine: 43, endOffset: 12 } + } + ] + }, + { + locations: [ + { + component: 'bar', + textRange: { startLine: 43, startOffset: 6, endLine: 43, endOffset: 8 } + } + ] + }, + { + locations: [ + { + component: 'foo', + textRange: { startLine: 70, startOffset: 12, endLine: 70, endOffset: 16 } + } + ] + } ] } as any; - expect(parseIssueFromResponse(issue).secondaryLocations).toEqual([ - { textRange: { startLine: 43, startOffset: 6, endLine: 43, endOffset: 8 } }, - { textRange: { startLine: 43, startOffset: 8, endLine: 43, endOffset: 12 } }, - { textRange: { startLine: 68, startOffset: 5, endLine: 68, endOffset: 7 } }, - { textRange: { startLine: 70, startOffset: 12, endLine: 70, endOffset: 16 } } + const components = [{ key: 'foo', name: 'src/foo.js' }, { key: 'bar', name: 'src/bar.js' }]; + expect(parseIssueFromResponse(issue, components).secondaryLocations).toEqual([ + { + component: 'bar', + componentName: 'src/bar.js', + textRange: { endLine: 43, endOffset: 8, startLine: 43, startOffset: 6 } + }, + { + component: 'unknown', + componentName: undefined, + textRange: { endLine: 43, endOffset: 12, startLine: 43, startOffset: 8 } + }, + { + component: 'foo', + componentName: 'src/foo.js', + textRange: { endLine: 68, endOffset: 7, startLine: 68, startOffset: 5 } + }, + { + component: 'foo', + componentName: 'src/foo.js', + textRange: { endLine: 70, endOffset: 16, startLine: 70, startOffset: 12 } + } ]); }); diff --git a/server/sonar-web/src/main/js/helpers/issues.ts b/server/sonar-web/src/main/js/helpers/issues.ts index a1ccedd665c..3ea7c238628 100644 --- a/server/sonar-web/src/main/js/helpers/issues.ts +++ b/server/sonar-web/src/main/js/helpers/issues.ts @@ -19,19 +19,7 @@ */ import { flatten, sortBy } from 'lodash'; import { SEVERITIES } from './constants'; -import { Issue } from '../app/types'; - -interface TextRange { - startLine: number; - endLine: number; - startOffset: number; - endOffset: number; -} - -interface FlowLocation { - msg: string; - textRange?: TextRange; -} +import { Issue, FlowLocation, TextRange, Omit } from '../app/types'; interface Comment { login: string; @@ -44,7 +32,10 @@ interface User { interface Rule {} -interface Component {} +interface Component { + key: string; + name: string; +} interface IssueBase { severity: string; @@ -57,7 +48,8 @@ export interface RawIssue extends IssueBase { comments?: Array; component: string; flows?: Array<{ - locations?: FlowLocation[]; + // `componentName` is not available in RawIssue + locations?: Array>; }>; key: string; line?: number; @@ -132,11 +124,18 @@ function reverseLocations(locations: FlowLocation[]): FlowLocation[] { } function splitFlows( - issue: RawIssue + issue: RawIssue, + components: Component[] = [] ): { secondaryLocations: FlowLocation[]; flows: FlowLocation[][] } { - const parsedFlows = (issue.flows || []) - .filter(flow => flow.locations != null) - .map(flow => flow.locations!.filter(location => location.textRange != null)); + const parsedFlows: FlowLocation[][] = (issue.flows || []) + .filter(flow => flow.locations !== undefined) + .map(flow => flow.locations!.filter(location => location.textRange != null)) + .map(flow => + flow.map(location => { + const component = components.find(component => component.key === location.component); + return { ...location, componentName: component && component.name }; + }) + ); const onlySecondaryLocations = parsedFlows.every(flow => flow.length === 1); @@ -159,7 +158,7 @@ export function parseIssueFromResponse( users?: User[], rules?: Rule[] ): Issue { - const { secondaryLocations, flows } = splitFlows(issue); + const { secondaryLocations, flows } = splitFlows(issue, components); return { ...issue, ...injectRelational(issue, components, 'component', 'key'), diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties index 7e76dfebc6f..397cb2e3bd7 100644 --- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties +++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties @@ -620,6 +620,7 @@ issues.to_switch_flows=to switch flows issues.leak_period=Leak Period issues.my_issues=My Issues issues.no_my_issues=There are no issues assigned to you. +issues.x_more_locations=+ {0} more location(s) #------------------------------------------------------------------------------