-
Notifications
You must be signed in to change notification settings - Fork 733
ci(jscpd): check changed lines instead of changed files to avoid false positives #5950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
451a58f
implement script for parsing diff
Hweinstock a82f098
switch to single-script approach
Hweinstock b3d5b6b
Merge branch 'master' into jscpd/changedlines
Hweinstock 86da344
add tests for script
Hweinstock 26f71be
manually insert copy
Hweinstock d960c62
insert another copy
Hweinstock 89472ee
undo manual copies
Hweinstock 171392a
Merge branch 'master' into jscpd/changedlines
Hweinstock ad81faf
Merge branch 'master' into jscpd/changedLines
Hweinstock e5e175f
Merge branch 'master' into jscpd/changedlines
Hweinstock 6cf7bcc
Merge branch 'master' into jscpd/changedlines
Hweinstock fffd3b2
merge in master
Hweinstock 12fc4d6
add clone manually
Hweinstock 69a4309
remove clone
Hweinstock 1051963
add info str and centralize test file
Hweinstock 662adc2
fix typo
Hweinstock File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,268 @@ | ||
| const fs = require('fs/promises') | ||
| const path = require('path') | ||
|
|
||
| function parseDiffFilePath(filePathLine) { | ||
| return filePathLine.split(' ')[2].split('/').slice(1).join('/') | ||
| } | ||
|
|
||
| function parseDiffRange(rangeLine) { | ||
| const [_fromRange, toRange] = rangeLine.split(' ').slice(1, 3) | ||
| const [startLine, numLines] = toRange.slice(1).split(',').map(Number) | ||
| const range = [startLine, startLine + numLines] | ||
| return range | ||
| } | ||
|
|
||
| async function parseDiff(diffPath) { | ||
| const diff = await fs.readFile(diffPath, 'utf8') | ||
| const lines = diff.split('\n') | ||
| let currentFile = null | ||
| let currentFileChanges = [] | ||
| const fileChanges = new Map() | ||
|
|
||
| for (const line of lines) { | ||
| if (line.startsWith('diff')) { | ||
| if (currentFile) { | ||
| fileChanges.set(currentFile, currentFileChanges) | ||
| } | ||
| currentFile = parseDiffFilePath(line) | ||
| currentFileChanges = [] | ||
| } | ||
| if (line.startsWith('@@')) { | ||
| currentFileChanges.push(parseDiffRange(line)) | ||
| } | ||
| } | ||
|
|
||
| fileChanges.set(currentFile, currentFileChanges) | ||
|
|
||
| return fileChanges | ||
| } | ||
|
|
||
| function doesOverlap(range1, range2) { | ||
| const [start1, end1] = range1 | ||
| const [start2, end2] = range2 | ||
| return ( | ||
| (start1 >= start2 && start1 <= end2) || (end1 >= start2 && end1 <= end2) || (start2 >= start1 && end2 <= end1) | ||
| ) | ||
| } | ||
|
|
||
| function isCloneInChanges(changes, cloneInstance) { | ||
| const fileName = cloneInstance.name | ||
| const cloneStart = cloneInstance.start | ||
| const cloneEnd = cloneInstance.end | ||
| const lineChangeRanges = changes.get(fileName) | ||
|
|
||
| if (!lineChangeRanges) { | ||
| return false | ||
| } | ||
|
|
||
| return lineChangeRanges.some((range) => doesOverlap([cloneStart, cloneEnd], range)) | ||
| } | ||
|
|
||
| function isInChanges(changes, dupe) { | ||
| return isCloneInChanges(changes, dupe.firstFile) || isCloneInChanges(changes, dupe.secondFile) | ||
| } | ||
|
|
||
| function filterDuplicates(report, changes) { | ||
| duplicates = [] | ||
| for (const dupe of report.duplicates) { | ||
| if (isInChanges(changes, dupe)) { | ||
| duplicates.push(dupe) | ||
| } | ||
| } | ||
| return duplicates | ||
| } | ||
|
|
||
| async function run() { | ||
| const rawDiffPath = process.argv[3] | ||
| const jscpdReportPath = process.argv[4] | ||
| const changes = await parseDiff(rawDiffPath) | ||
| const jscpdReport = JSON.parse(await fs.readFile(jscpdReportPath, 'utf8')) | ||
| const filteredDuplicates = filterDuplicates(jscpdReport, changes) | ||
|
|
||
| console.log(filteredDuplicates) | ||
| console.log('%s files changes', changes.size) | ||
| console.log('%s duplicates found', filteredDuplicates.length) | ||
| if (filteredDuplicates.length > 0) { | ||
| process.exit(1) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Mini-test Suite | ||
| */ | ||
| let testCounter = 0 | ||
| function assertEqual(actual, expected) { | ||
| if (actual !== expected) { | ||
| throw new Error(`Expected ${expected} but got ${actual}`) | ||
| } | ||
| testCounter += 1 | ||
| } | ||
|
|
||
| async function test() { | ||
justinmk3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| test_parseDiffFilePath() | ||
| test_parseDiffRange() | ||
| test_doesOverlap() | ||
| await test_parseDiff() | ||
| await test_isCloneInChanges() | ||
| await test_isInChanges() | ||
| await test_filterDuplicates() | ||
| console.log('All tests passed (%s)', testCounter) | ||
| } | ||
|
|
||
| function test_parseDiffFilePath() { | ||
| assertEqual( | ||
| parseDiffFilePath( | ||
| 'diff --git a/.github/workflows/copyPasteDetection.yml b/.github/workflows/copyPasteDetection.yml' | ||
| ), | ||
| '.github/workflows/copyPasteDetection.yml' | ||
| ) | ||
| assertEqual( | ||
| parseDiffFilePath('diff --git a/.github/workflows/filterDuplicates.js b/.github/workflows/filterDuplicates.js'), | ||
| '.github/workflows/filterDuplicates.js' | ||
| ) | ||
| } | ||
|
|
||
| function test_parseDiffRange() { | ||
| assertEqual(parseDiffRange('@@ -1,4 +1,4 @@').join(','), '1,5') | ||
| assertEqual(parseDiffRange('@@ -10,4 +10,4 @@').join(','), '10,14') | ||
| assertEqual(parseDiffRange('@@ -10,4 +10,5 @@').join(','), '10,15') | ||
| } | ||
|
|
||
| function test_doesOverlap() { | ||
| assertEqual(doesOverlap([1, 5], [2, 4]), true) | ||
| assertEqual(doesOverlap([2, 3], [2, 4]), true) | ||
| assertEqual(doesOverlap([2, 3], [1, 4]), true) | ||
| assertEqual(doesOverlap([1, 5], [5, 6]), true) | ||
| assertEqual(doesOverlap([1, 5], [6, 7]), false) | ||
| assertEqual(doesOverlap([6, 7], [1, 5]), false) | ||
| assertEqual(doesOverlap([2, 5], [4, 5]), true) | ||
| } | ||
|
|
||
| async function test_parseDiff() { | ||
| const testDiff = '.github/workflows/test/test_diff.txt' | ||
| const changes = await parseDiff(path.resolve(testDiff)) | ||
| assertEqual(changes.size, 2) | ||
| assertEqual(changes.get('.github/workflows/copyPasteDetection.yml').length, 1) | ||
| assertEqual(changes.get('.github/workflows/filterDuplicates.js').length, 1) | ||
| assertEqual(changes.get('.github/workflows/filterDuplicates.js')[0].join(','), '1,86') | ||
| assertEqual(changes.get('.github/workflows/copyPasteDetection.yml')[0].join(','), '26,73') | ||
| } | ||
|
|
||
| async function test_isCloneInChanges() { | ||
| const testDiff = '.github/workflows/test/test_diff.txt' | ||
| const changes = await parseDiff(path.resolve(testDiff)) | ||
| assertEqual( | ||
| isCloneInChanges(changes, { | ||
| name: '.github/workflows/filterDuplicates.js', | ||
| start: 1, | ||
| end: 86, | ||
| }), | ||
| true | ||
| ) | ||
| assertEqual( | ||
| isCloneInChanges(changes, { | ||
| name: '.github/workflows/filterDuplicates.js', | ||
| start: 80, | ||
| end: 95, | ||
| }), | ||
| true | ||
| ) | ||
| assertEqual( | ||
| isCloneInChanges(changes, { | ||
| name: '.github/workflows/filterDuplicates.js', | ||
| start: 87, | ||
| end: 95, | ||
| }), | ||
| false | ||
| ) | ||
| assertEqual( | ||
| isCloneInChanges(changes, { | ||
| name: 'some-fake-file', | ||
| start: 1, | ||
| end: 100, | ||
| }), | ||
| false | ||
| ) | ||
| } | ||
|
|
||
| async function test_isInChanges() { | ||
| const testDiff = '.github/workflows/test/test_diff.txt' | ||
| const changes = await parseDiff(path.resolve(testDiff)) | ||
| const dupe = { | ||
| firstFile: { | ||
| name: '.github/workflows/filterDuplicates.js', | ||
| start: 1, | ||
| end: 86, | ||
| }, | ||
| secondFile: { | ||
| name: '.github/workflows/filterDuplicates.js', | ||
| start: 80, | ||
| end: 95, | ||
| }, | ||
| } | ||
| assertEqual(isInChanges(changes, dupe), true) | ||
| dupe.secondFile.start = 87 | ||
| assertEqual(isInChanges(changes, dupe), true) | ||
| dupe.firstFile.name = 'some-fake-file' | ||
| assertEqual(isInChanges(changes, dupe), false) | ||
| } | ||
|
|
||
| async function test_filterDuplicates() { | ||
| assertEqual( | ||
| filterDuplicates( | ||
| { | ||
| duplicates: [ | ||
| { | ||
| firstFile: { | ||
| name: '.github/workflows/filterDuplicates.js', | ||
| start: 1, | ||
| end: 86, | ||
| }, | ||
| secondFile: { | ||
| name: '.github/workflows/filterDuplicates.js', | ||
| start: 80, | ||
| end: 95, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| await parseDiff('.github/workflows/test/test_diff.txt') | ||
| ).length, | ||
| 1 | ||
| ) | ||
| assertEqual( | ||
| filterDuplicates( | ||
| { | ||
| duplicates: [ | ||
| { | ||
| firstFile: { | ||
| name: 'some-other-file', | ||
| start: 1, | ||
| end: 86, | ||
| }, | ||
| secondFile: { | ||
| name: '.github/workflows/filterDuplicates.js', | ||
| start: 90, | ||
| end: 95, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| await parseDiff('.github/workflows/test/test_diff.txt') | ||
| ).length, | ||
| 0 | ||
| ) | ||
| } | ||
|
|
||
| async function main() { | ||
| const mode = process.argv[2] | ||
| if (mode === 'run') { | ||
| await run() | ||
| } else if (mode === 'test') { | ||
| await test() | ||
| } else { | ||
| throw new Error('Invalid mode') | ||
| } | ||
| } | ||
|
|
||
| void main() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.