]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-13260 Add primary location component when missing
authorJeremy Davis <jeremy.davis@sonarsource.com>
Fri, 17 Apr 2020 12:11:01 +0000 (14:11 +0200)
committersonartech <sonartech@sonarsource.com>
Wed, 22 Apr 2020 20:03:36 +0000 (20:03 +0000)
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/ComponentSourceSnippetViewer.tsx
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/CrossComponentSourceViewerWrapper.tsx
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/ComponentSourceSnippetViewer-test.tsx
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/__tests__/utils-test.ts
server/sonar-web/src/main/js/apps/issues/crossComponentSourceViewer/utils.ts

index 5519d70d6b32d9d0c5cabcf542d1bbeab1949238..c3bc8885b47517cef3e09f9c21c977f032304696 100644 (file)
@@ -88,10 +88,13 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
   }
 
   createSnippetsFromProps() {
+    const { issue, snippetGroup } = this.props;
+
     const snippets = createSnippets({
-      locations: this.props.snippetGroup.locations,
-      issue: this.props.issue,
-      addIssueLocation: this.props.issue.secondaryLocations.length > 0
+      locations: snippetGroup.locations,
+      issue,
+      addIssueLocation:
+        issue.secondaryLocations.length > 0 && snippetGroup.component.key === issue.component
     });
 
     this.setState({ snippets });
@@ -354,7 +357,8 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
   render() {
     const { branchLike, duplications, issue, issuesByLine, last, snippetGroup } = this.props;
     const { additionalLines, loading, snippets } = this.state;
-    const locations = locationsByLine([issue]);
+    const locations =
+      issue.component === snippetGroup.component.key ? locationsByLine([issue]) : {};
 
     const fullyShown =
       snippets.length === 1 &&
@@ -367,6 +371,10 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
       ...additionalLines
     });
 
+    const isFlow = issue.secondaryLocations.length === 0;
+    const includeIssueLocation = (index: number) =>
+      isFlow ? last && index === snippets.length - 1 : index === 0;
+
     return (
       <div
         className={classNames('component-source-container', {
@@ -385,8 +393,8 @@ export default class ComponentSourceSnippetViewer extends React.PureComponent<Pr
             {this.renderSnippet({
               snippet,
               index: snippets[index].index,
-              issuesByLine: last ? issuesByLine : {},
-              locationsByLine: last && index === snippets.length - 1 ? locations : {},
+              issuesByLine,
+              locationsByLine: includeIssueLocation(index) ? locations : {},
               last: last && index === snippets.length - 1
             })}
           </div>
index 0fd57eb1956da8e007ae8b36bb616b63ded9fbdd..2326b711d7e6930e69903132ddcf978382bf8606 100644 (file)
@@ -31,7 +31,7 @@ import {
   isDuplicationBlockInRemovedComponent
 } from '../../../components/SourceViewer/helpers/duplications';
 import {
-  duplicationsByLine,
+  duplicationsByLine as getDuplicationsByLine,
   issuesByComponentAndLine
 } from '../../../components/SourceViewer/helpers/indexing';
 import { SourceViewerContext } from '../../../components/SourceViewer/SourceViewerContext';
@@ -99,7 +99,7 @@ export default class CrossComponentSourceViewerWrapper extends React.PureCompone
           this.setState(state => ({
             duplicatedFiles: r.files,
             duplications: r.duplications,
-            duplicationsByLine: duplicationsByLine(r.duplications),
+            duplicationsByLine: getDuplicationsByLine(r.duplications),
             linePopup:
               r.duplications.length === 1
                 ? { component, index: 0, line: line.line, name: 'duplications' }
@@ -223,9 +223,10 @@ export default class CrossComponentSourceViewerWrapper extends React.PureCompone
       );
     }
 
+    const { issue, locations } = this.props;
     const { components, duplications, duplicationsByLine, linePopup } = this.state;
     const issuesByComponent = issuesByComponentAndLine(this.props.issues);
-    const locationsByComponent = groupLocationsByComponent(this.props.locations, components);
+    const locationsByComponent = groupLocationsByComponent(issue, locations, components);
 
     return (
       <div>
@@ -240,12 +241,13 @@ export default class CrossComponentSourceViewerWrapper extends React.PureCompone
           }
           return (
             <SourceViewerContext.Provider
-              key={`${this.props.issue.key}-${this.props.selectedFlowIndex || 0}-${i}`}
+              // eslint-disable-next-line react/no-array-index-key
+              key={`${issue.key}-${this.props.selectedFlowIndex || 0}-${i}`}
               value={{ branchLike: this.props.branchLike, file: snippetGroup.component }}>
               <ComponentSourceSnippetViewer
                 branchLike={this.props.branchLike}
                 highlightedLocationMessage={this.props.highlightedLocationMessage}
-                issue={this.props.issue}
+                issue={issue}
                 issuePopup={this.state.issuePopup}
                 issuesByLine={issuesByComponent[snippetGroup.component.key] || {}}
                 last={i === locationsByComponent.length - 1}
index bf878ef92d1944ff7df097cf1c75bd3ed12a16a5..f9a76026d58a7c56d43202a874ed6277b24189cb 100644 (file)
@@ -54,15 +54,19 @@ it('should render correctly with secondary locations', () => {
   const snippetGroup: T.SnippetGroup = {
     locations: [
       mockFlowLocation({
-        component: 'a',
+        component: issue.component,
         textRange: { startLine: 34, endLine: 34, startOffset: 0, endOffset: 0 }
       }),
       mockFlowLocation({
-        component: 'a',
+        component: issue.component,
         textRange: { startLine: 74, endLine: 74, startOffset: 0, endOffset: 0 }
       })
     ],
-    ...mockSnippetsByComponent('a', [...range(2, 17), ...range(29, 39), ...range(69, 79)])
+    ...mockSnippetsByComponent(issue.component, [
+      ...range(2, 17),
+      ...range(29, 39),
+      ...range(69, 79)
+    ])
   };
   const wrapper = shallowRender({ issue, snippetGroup });
   expect(wrapper.state('snippets')).toHaveLength(3);
@@ -71,6 +75,36 @@ it('should render correctly with secondary locations', () => {
   expect(wrapper.state('snippets')[2]).toEqual({ index: 2, start: 69, end: 79 });
 });
 
+it('should render correctly with flows', () => {
+  // issue with flows but no secondary locations
+  const issue = mockIssue(true, {
+    secondaryLocations: [],
+    textRange: { startLine: 7, endLine: 7, startOffset: 5, endOffset: 10 }
+  });
+
+  const snippetGroup: T.SnippetGroup = {
+    locations: [
+      mockFlowLocation({
+        component: issue.component,
+        textRange: { startLine: 34, endLine: 34, startOffset: 0, endOffset: 0 }
+      }),
+      mockFlowLocation({
+        component: issue.component,
+        textRange: { startLine: 74, endLine: 74, startOffset: 0, endOffset: 0 }
+      })
+    ],
+    ...mockSnippetsByComponent(issue.component, [
+      ...range(2, 17),
+      ...range(29, 39),
+      ...range(69, 79)
+    ])
+  };
+  const wrapper = shallowRender({ issue, snippetGroup });
+  expect(wrapper.state('snippets')).toHaveLength(2);
+  expect(wrapper.state('snippets')[0]).toEqual({ index: 0, start: 29, end: 39 });
+  expect(wrapper.state('snippets')[1]).toEqual({ index: 1, start: 69, end: 79 });
+});
+
 it('should expand block', async () => {
   (getSources as jest.Mock).mockResolvedValueOnce(
     Object.values(mockSnippetsByComponent('a', range(6, 59)).sources)
index 70408a0207905d102dcc71377eaec33474305ac2..6c6d15d9d49e6e7f9431796ca7598a02a3ea16f6 100644 (file)
@@ -26,11 +26,12 @@ import { createSnippets, expandSnippet, groupLocationsByComponent } from '../uti
 
 describe('groupLocationsByComponent', () => {
   it('should handle empty args', () => {
-    expect(groupLocationsByComponent([], {})).toEqual([]);
+    expect(groupLocationsByComponent(mockIssue(), [], {})).toEqual([]);
   });
 
   it('should group correctly', () => {
     const results = groupLocationsByComponent(
+      mockIssue(),
       [
         mockFlowLocation({
           textRange: { startLine: 16, startOffset: 10, endLine: 16, endOffset: 14 }
@@ -50,6 +51,7 @@ describe('groupLocationsByComponent', () => {
 
   it('should preserve step order when jumping between files', () => {
     const results = groupLocationsByComponent(
+      mockIssue(),
       [
         mockFlowLocation({
           component: 'A.js',
index 12b5629fca501306093cbaeaac5ca09792a4a536..1a778699d9a5ac185a9c59720881fe22e431a78a 100644 (file)
@@ -42,7 +42,7 @@ function collision([startA, endA]: number[], [startB, endB]: number[]) {
   return !(startA > endB + MERGE_DISTANCE || endA < startB - MERGE_DISTANCE);
 }
 
-function getPrimaryLocation(issue: T.Issue): T.FlowLocation {
+export function getPrimaryLocation(issue: T.Issue): T.FlowLocation {
   return {
     component: issue.component,
     textRange: issue.textRange || {
@@ -128,6 +128,7 @@ export function linesForSnippets(snippets: T.Snippet[], componentLines: T.LineMa
 }
 
 export function groupLocationsByComponent(
+  issue: T.Issue,
   locations: T.FlowLocation[],
   components: { [key: string]: T.SnippetsByComponent }
 ) {
@@ -135,14 +136,25 @@ export function groupLocationsByComponent(
   let currentGroup: T.SnippetGroup;
   const groups: T.SnippetGroup[] = [];
 
+  const addGroup = (loc: T.FlowLocation) => {
+    currentGroup = {
+      ...(components[loc.component] || unknownComponent(loc.component)),
+      locations: []
+    };
+    groups.push(currentGroup);
+    currentComponent = loc.component;
+  };
+
+  if (
+    issue.secondaryLocations.length > 0 &&
+    locations.every(loc => loc.component !== issue.component)
+  ) {
+    addGroup(getPrimaryLocation(issue));
+  }
+
   locations.forEach((loc, index) => {
     if (loc.component !== currentComponent) {
-      currentGroup = {
-        ...(components[loc.component] || unknownComponent(loc.component)),
-        locations: []
-      };
-      groups.push(currentGroup);
-      currentComponent = loc.component;
+      addGroup(loc);
     }
     loc.index = index;
     currentGroup.locations.push(loc);