diff options
author | Guillaume Peoc'h <guillaume.peoch@sonarsource.com> | 2022-08-19 17:32:00 +0200 |
---|---|---|
committer | sonartech <sonartech@sonarsource.com> | 2022-08-24 20:03:40 +0000 |
commit | 4e7ba7e8b67a22abd7f29da6af178525958a1509 (patch) | |
tree | 26f47966fd0202c6e76222b62e112034ec33a2c3 /server/sonar-web | |
parent | 0ef79cc3e2fbedce2e6209ed05e9cbf5ceabdda6 (diff) | |
download | sonarqube-4e7ba7e8b67a22abd7f29da6af178525958a1509.tar.gz sonarqube-4e7ba7e8b67a22abd7f29da6af178525958a1509.zip |
SONAR-17201 Display LINE differences between compliant and noncompliant code
Diffstat (limited to 'server/sonar-web')
8 files changed, 360 insertions, 8 deletions
diff --git a/server/sonar-web/package.json b/server/sonar-web/package.json index 9122f528e8a..0c4c2d124e4 100644 --- a/server/sonar-web/package.json +++ b/server/sonar-web/package.json @@ -7,6 +7,7 @@ "dependencies": { "@emotion/react": "11.10.0", "@emotion/styled": "11.10.0", + "@types/diff": "5.0.2", "classnames": "2.3.1", "clipboard": "2.0.11", "core-js": "3.21.1", @@ -17,6 +18,7 @@ "d3-shape": "1.3.7", "d3-zoom": "1.8.3", "date-fns": "2.29.2", + "diff": "5.1.0", "dompurify": "2.3.10", "formik": "1.2.0", "lodash": "4.17.21", diff --git a/server/sonar-web/src/main/js/app/styles/style.css b/server/sonar-web/src/main/js/app/styles/style.css index cb1136bd639..9be5f3f5780 100644 --- a/server/sonar-web/src/main/js/app/styles/style.css +++ b/server/sonar-web/src/main/js/app/styles/style.css @@ -150,12 +150,33 @@ } .rule-desc pre, -.markdown pre { - padding: 10px; - border-top: 1px solid var(--barBorderColor); - border-bottom: 1px solid var(--barBorderColor); - line-height: 18px; - overflow: auto; +.markdown pre, +.code-difference { + background-color: var(--codeBackground); + border-radius: 8px; + border: 1px solid var(--codeBorder); + padding: calc(2 * var(--gridSize)); +} + +.code-difference pre { + padding: 0; + border: none; +} + +.code-difference .code-added { + background-color: var(--codeAdded); + padding-left: calc(2 * var(--gridSize)); + margin-left: calc(-2 * var(--gridSize)); + margin-right: calc(-2 * var(--gridSize)); + border-radius: 0; +} + +.code-difference .code-removed { + background-color: var(--codeRemoved); + padding-left: calc(2 * var(--gridSize)); + margin-left: calc(-2 * var(--gridSize)); + margin-right: calc(-2 * var(--gridSize)); + border-radius: 0; } .rule-desc code, diff --git a/server/sonar-web/src/main/js/app/theme.js b/server/sonar-web/src/main/js/app/theme.js index 7e61fdb060c..2dd5ec2747e 100644 --- a/server/sonar-web/src/main/js/app/theme.js +++ b/server/sonar-web/src/main/js/app/theme.js @@ -131,7 +131,10 @@ module.exports = { github: '#e1e4e8', // code/pre - codeBackground: '#e6e6e6', + codeBackground: '#f5f5f5', + codeBorder: '#e6e6e6', + codeAdded: '#dff0d8', + codeRemoved: '#f2dede', //promotion darkBackground: '#292929', diff --git a/server/sonar-web/src/main/js/components/rules/RuleDescription.tsx b/server/sonar-web/src/main/js/components/rules/RuleDescription.tsx index a3f29e3b9ff..90767868178 100644 --- a/server/sonar-web/src/main/js/components/rules/RuleDescription.tsx +++ b/server/sonar-web/src/main/js/components/rules/RuleDescription.tsx @@ -20,6 +20,7 @@ import classNames from 'classnames'; import * as React from 'react'; import { RuleDescriptionSection } from '../../apps/coding-rules/rule'; +import applyCodeDifferences from '../../helpers/code-difference'; import { translate, translateWithParameters } from '../../helpers/l10n'; import { sanitizeString } from '../../helpers/sanitize'; import ButtonToggle from '../controls/ButtonToggle'; @@ -123,7 +124,10 @@ export default class RuleDescription extends React.PureComponent<Props, State> { className={classNames(className, { markdown: isDefault, 'rule-desc': !isDefault - })}> + })} + ref={node => { + applyCodeDifferences(node); + }}> <div className="rules-context-description"> <h2 className="rule-contexts-title"> {translate('coding_rules.description_context.title')} @@ -171,6 +175,9 @@ export default class RuleDescription extends React.PureComponent<Props, State> { markdown: isDefault, 'rule-desc': !isDefault })} + ref={node => { + applyCodeDifferences(node); + }} // eslint-disable-next-line react/no-danger dangerouslySetInnerHTML={{ __html: sanitizeString(sections[0].content) diff --git a/server/sonar-web/src/main/js/helpers/__tests__/__snapshots__/code-difference-test.tsx.snap b/server/sonar-web/src/main/js/helpers/__tests__/__snapshots__/code-difference-test.tsx.snap new file mode 100644 index 00000000000..3234983cc37 --- /dev/null +++ b/server/sonar-web/src/main/js/helpers/__tests__/__snapshots__/code-difference-test.tsx.snap @@ -0,0 +1,57 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`should apply diff view correctly: differenciatedCode 1`] = ` +HTMLCollection [ + <div + class="code-difference" + > + <pre> + public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException +{ + String data = request.getParameter("input"); + PrintWriter writer = response.getWriter(); + + + </pre> + <pre + class="code-removed" + > + writer.print(data); // Noncompliant + + </pre> + <pre> + } + + </pre> + </div>, + <div + class="code-difference" + > + <pre + class="code-added" + > + import org.owasp.encoder.Encode; + + + </pre> + <pre> + public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException +{ + String data = request.getParameter("input"); + PrintWriter writer = response.getWriter(); + + + </pre> + <pre + class="code-added" + > + writer.print(Encode.forHtml(data)); + + </pre> + <pre> + } + + </pre> + </div>, +] +`; diff --git a/server/sonar-web/src/main/js/helpers/__tests__/code-difference-test.tsx b/server/sonar-web/src/main/js/helpers/__tests__/code-difference-test.tsx new file mode 100644 index 00000000000..662deda1bcd --- /dev/null +++ b/server/sonar-web/src/main/js/helpers/__tests__/code-difference-test.tsx @@ -0,0 +1,152 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 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 { render } from '@testing-library/react'; +import React from 'react'; +import applyCodeDifferences from '../code-difference'; + +it('should apply diff view correctly', () => { + const { container } = renderDom(properCodeSnippet); + applyCodeDifferences(container); + expect(container.getElementsByClassName('code-difference')).toMatchSnapshot('differenciatedCode'); +}); + +it('should not apply diff view if 3 examples are present', () => { + const { container } = renderDom(codeSnippetWith3Examples); + applyCodeDifferences(container); + expect(container.getElementsByClassName('code-difference').length).toEqual(0); +}); + +it('should not apply diff view if compliant code is absent', () => { + const { container } = renderDom(codeSnippetWithoutCompliantCode); + applyCodeDifferences(container); + expect(container.getElementsByClassName('code-difference').length).toEqual(0); +}); + +const properCodeSnippet = `<!DOCTYPE html><body> +<h1>Some title</h1> +<p>Some paragraph...</p> + +<h2>Example 1</h2> + +<pre data-diff-id="1" data-diff-type="noncompliant"> +public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException +{ + String data = request.getParameter("input"); + PrintWriter writer = response.getWriter(); + + writer.print(data); // Noncompliant +} +</pre> + +<p>Some other paragraph</p> + +<pre data-diff-id="1" data-diff-type="compliant"> +import org.owasp.encoder.Encode; + +public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException +{ + String data = request.getParameter("input"); + PrintWriter writer = response.getWriter(); + + writer.print(Encode.forHtml(data)); +} +</pre> + +<p>Final paragraph</p> +</body></html>`; + +const codeSnippetWith3Examples = `<!DOCTYPE html><body> +<h1>Some title</h1> +<p>Some paragraph...</p> + +<h2>Example 1</h2> + +<pre data-diff-id="1" data-diff-type="noncompliant"> +public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException +{ + String data = request.getParameter("input"); + PrintWriter writer = response.getWriter(); + + writer.print(data); // Noncompliant +} +</pre> + +<p>Some other paragraph</p> + +<pre data-diff-id="1" data-diff-type="compliant"> +import org.owasp.encoder.Encode; + +public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException +{ + String data = request.getParameter("input"); + PrintWriter writer = response.getWriter(); + + writer.print(Encode.forHtml(data)); +} +</pre> + +<p>Some other paragraph</p> + +<pre data-diff-id="1" data-diff-type="compliant"> +import org.owasp.encoder.Encode; + +public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException +{ + String data = request.getParameter("input"); + PrintWriter writer = response.getWriter(); + + writer.print(Encode.forHtml(data)); +} +</pre> + +<p>Final paragraph</p> +</body></html>`; + +const codeSnippetWithoutCompliantCode = `<!DOCTYPE html><body> +<h1>Some title</h1> +<p>Some paragraph...</p> + +<h2>Example 1</h2> + +<pre data-diff-id="1" data-diff-type="noncompliant"> +public void endpoint(HttpServletRequest request, HttpServletResponse response) throws IOException +{ + String data = request.getParameter("input"); + PrintWriter writer = response.getWriter(); + + writer.print(data); // Noncompliant +} +</pre> + +<p>Some other paragraph</p> +<p>Final paragraph</p> +</body></html>`; + +function renderDom(codeSnippet: string) { + return render( + <div + className="markdown" + // eslint-disable-next-line react/no-danger + dangerouslySetInnerHTML={{ + __html: codeSnippet + }} + /> + ); +} diff --git a/server/sonar-web/src/main/js/helpers/code-difference.ts b/server/sonar-web/src/main/js/helpers/code-difference.ts new file mode 100644 index 00000000000..140ddb7ac87 --- /dev/null +++ b/server/sonar-web/src/main/js/helpers/code-difference.ts @@ -0,0 +1,94 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 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 { diffLines } from 'diff'; +import { groupBy, keyBy } from 'lodash'; + +const NUMBER_OF_EXAMPLES = 2; + +type DiffBlock = { noncompliant: Element; compliant: Element }; + +export default function applyCodeDifferences(element: Element | null) { + if (element === null) { + return; + } + const codeExamples = getExamplesFromDom(element); + + codeExamples.forEach(({ noncompliant, compliant }) => { + if (noncompliant === undefined || compliant === undefined) { + return; + } + const [markedNonCompliant, markedCompliantCode] = differentiateCode( + noncompliant.innerHTML, + compliant.innerHTML + ); + + replaceInDom(noncompliant, markedNonCompliant); + replaceInDom(compliant, markedCompliantCode); + }); +} + +function getExamplesFromDom(element: Element) { + const pres = Array.from(element.querySelectorAll(`pre[data-diff-id]`)); + + return ( + Object.values( + groupBy( + pres.filter(e => e.getAttribute('data-diff-id') !== undefined), + e => e.getAttribute('data-diff-id') + ) + ) + // If we have 1 or 3+ example we can't display any differences + .filter(diffsBlock => diffsBlock.length === NUMBER_OF_EXAMPLES) + .map( + diffBlock => keyBy(diffBlock, block => block.getAttribute('data-diff-type')) as DiffBlock + ) + ); +} + +function differentiateCode(compliant: string, nonCompliant: string) { + const hunks = diffLines(compliant, nonCompliant); + + let nonCompliantCode = ''; + let compliantCode = ''; + + hunks.forEach(hunk => { + if (!hunk.added && !hunk.removed) { + nonCompliantCode += `<pre>${hunk.value}</pre>`; + compliantCode += `<pre>${hunk.value}</pre>`; + } + + if (hunk.added) { + compliantCode += `<pre class='code-added'>${hunk.value}</pre>`; + } + + if (hunk.removed) { + nonCompliantCode += `<pre class='code-removed'>${hunk.value}</pre>`; + } + }); + return [nonCompliantCode, compliantCode]; +} + +function replaceInDom(current: Element, code: string) { + const markedCode = document.createElement('div'); + markedCode.classList.add('code-difference'); + markedCode.innerHTML = code; + current.parentNode?.replaceChild(markedCode, current); +} diff --git a/server/sonar-web/yarn.lock b/server/sonar-web/yarn.lock index 94d62542c2f..de40c1812c3 100644 --- a/server/sonar-web/yarn.lock +++ b/server/sonar-web/yarn.lock @@ -1832,6 +1832,13 @@ __metadata: languageName: node linkType: hard +"@types/diff@npm:5.0.2": + version: 5.0.2 + resolution: "@types/diff@npm:5.0.2" + checksum: 8fbc419b5aca33f494026bf5f70e026f76367689677ef114f9c078ac738d7dbe96e6dda3fd8290e4a7c35281e2b60b034e3d7e3c968b850cf06a21279e7ddcbe + languageName: node + linkType: hard + "@types/dompurify@npm:2.3.3": version: 2.3.3 resolution: "@types/dompurify@npm:2.3.3" @@ -2321,6 +2328,7 @@ __metadata: "@types/d3-selection": 1.3.2 "@types/d3-shape": 1.2.4 "@types/d3-zoom": 1.7.3 + "@types/diff": 5.0.2 "@types/dompurify": 2.3.3 "@types/enzyme": 3.10.5 "@types/jest": 27.4.1 @@ -2347,6 +2355,7 @@ __metadata: d3-shape: 1.3.7 d3-zoom: 1.8.3 date-fns: 2.29.2 + diff: 5.1.0 dompurify: 2.3.10 enzyme: 3.11.0 enzyme-adapter-react-16: 1.15.6 @@ -4003,6 +4012,13 @@ __metadata: languageName: node linkType: hard +"diff@npm:5.1.0": + version: 5.1.0 + resolution: "diff@npm:5.1.0" + checksum: c7bf0df7c9bfbe1cf8a678fd1b2137c4fb11be117a67bc18a0e03ae75105e8533dbfb1cda6b46beb3586ef5aed22143ef9d70713977d5fb1f9114e21455fba90 + languageName: node + linkType: hard + "dir-glob@npm:^3.0.1": version: 3.0.1 resolution: "dir-glob@npm:3.0.1" |