Skip to content
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

Cannot run detectInFiles() several times in a row #207

Closed
ehmicky opened this issue Feb 22, 2019 · 6 comments
Closed

Cannot run detectInFiles() several times in a row #207

ehmicky opened this issue Feb 22, 2019 · 6 comments
Labels
bug An issue contains information about wrong behaviour in progress

Comments

@ehmicky
Copy link

ehmicky commented Feb 22, 2019

Describe the bug
When running detectInFiles() several times in a row, the second run re-uses state from the previous run. This leads to files reporting duplication "on themselves" (i.e. duplicationA and duplicationB are the same line in the same file, for all lines in all files).

To Reproduce

example.js:

module.exports = {}

bug.js:

'use strict'

const { JSCPD } = require('jscpd')
const instance = new JSCPD({ minTokens: 1, minLines: 1 })
const detect = async function() {
  const clones = await instance.detectInFiles(['example.js'])
  console.log(clones)
}
detect().then(detect)

Output:

┌────────────┬────────────────┬─────────────┬──────────────┬──────────────────┬────┐
│ Format     │ Files analyzed │ Total lines │ Clones found │ Duplicated lines │ %  │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ javascript │ 1              │ 1           │ 0            │ 0                │ 0% │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ Total:     │ 1              │ 1           │ 0            │ 0                │ 0% │
└────────────┴────────────────┴─────────────┴──────────────┴──────────────────┴────┘
Execution Time: 91.967ms
[]
┌────────────┬────────────────┬─────────────┬──────────────┬──────────────────┬────┐
│ Format     │ Files analyzed │ Total lines │ Clones found │ Duplicated lines │ %  │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ javascript │ 2              │ 2           │ 0            │ 0                │ 0% │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ Total:     │ 2              │ 2           │ 0            │ 0                │ 0% │
└────────────┴────────────────┴─────────────┴──────────────┴──────────────────┴────┘
[ { format: 'javascript',
    foundDate: 1550844212331,
    duplicationA:
     { sourceId: '/home/user/Desktop/example.js',
       start: [Object],
       end: [Object],
       range: [Array],
       fragment: 'module.exports = {}' },
    duplicationB:
     { sourceId: '/home/user/Desktop/example.js',
       start: [Object],
       end: [Object],
       range: [Array],
       fragment: 'module.exports = {}' } } ]
(node:30646) Warning: No such label 'Execution Time' for console.timeEnd()

Expected behavior
No clones should be returned.

Possible cause
It seems like the store state is not cleaned between run. I'm not sure if this is intentional.

In jscpd.js line 244 there are two possible problems:

  • EventEmitter.emit() is synchronous with both core Node.js and eventemitter3. So it should be after EventEmitter.on(). This causes StoresManager.close() to never be closed.
  • EventEmitter.once() should probably be used instead. Otherwise re-running the main path will setup an additional end listener.

Desktop (please complete the following information):

  • OS: Ubuntu
  • OS Version: 18.10
  • NodeJS Version: 11.10.0
  • jscpd version: 2.0.9

Additional context
Note the (node:30646) Warning: No such label 'Execution Time' for console.timeEnd() at the end, which is probably a separate bug.

@ehmicky
Copy link
Author

ehmicky commented Feb 22, 2019

I just tried with 2.0.10 and now the output is:

┌────────────┬────────────────┬─────────────┬──────────────┬──────────────────┬────┐
│ Format     │ Files analyzed │ Total lines │ Clones found │ Duplicated lines │ %  │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ javascript │ 1              │ 1           │ 0            │ 0                │ 0% │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ Total:     │ 1              │ 1           │ 0            │ 0                │ 0% │
└────────────┴────────────────┴─────────────┴──────────────┴──────────────────┴────┘
Execution Time: 63.876ms
[]
┌────────────┬────────────────┬─────────────┬──────────────┬──────────────────┬────┐
│ Format     │ Files analyzed │ Total lines │ Clones found │ Duplicated lines │ %  │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ javascript │ 2              │ 2           │ 0            │ 0                │ 0% │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ Total:     │ 2              │ 2           │ 0            │ 0                │ 0% │
└────────────┴────────────────┴─────────────┴──────────────┴──────────────────┴────┘
[]
(node:31130) Warning: No such label 'Execution Time' for console.timeEnd()

It's better but I think Files analyzed should stay the same across runs, am I correct?

@ehmicky
Copy link
Author

ehmicky commented Feb 22, 2019

Another way to reproduce the issue with 2.0.10 and a single file bug.js:

'use strict'

const { JSCPD } = require('jscpd')
const instance = new JSCPD({ minTokens: 35 })
const jscpd = () => instance.detectInFiles(['bug.js'])
jscpd().then(jscpd)

Output:

┌────────────┬────────────────┬─────────────┬──────────────┬──────────────────┬────┐
│ Format     │ Files analyzed │ Total lines │ Clones found │ Duplicated lines │ %  │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ javascript │ 1              │ 6           │ 0            │ 0                │ 0% │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────┤
│ Total:     │ 1              │ 6           │ 0            │ 0                │ 0% │
└────────────┴────────────────┴─────────────┴──────────────┴──────────────────┴────┘
Execution Time: 59.605ms
Clone found (javascript):
 - bug.js [1:1 - 6:20]
   bug.js [1:1 - 6:20]

┌────────────┬────────────────┬─────────────┬──────────────┬──────────────────┬────────┐
│ Format     │ Files analyzed │ Total lines │ Clones found │ Duplicated lines │ %      │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────────┤
│ javascript │ 2              │ 12          │ 1            │ 5                │ 41.67% │
├────────────┼────────────────┼─────────────┼──────────────┼──────────────────┼────────┤
│ Total:     │ 2              │ 12          │ 1            │ 5                │ 41.67% │
└────────────┴────────────────┴─────────────┴──────────────┴──────────────────┴────────┘
(node:2562) Warning: No such label 'Execution Time' for console.timeEnd()

@ehmicky
Copy link
Author

ehmicky commented Feb 22, 2019

To put some context into this: the reason this is a problem is when jscpd is run in watch mode, e.g. through a task runner like Gulp.

@chul-hyun
Copy link

Temporary Solution

'use strict'

const { getStoreManager, JSCPD } = require('jscpd')
const instance = new JSCPD({ minTokens: 1, minLines: 1 })
const detect = async function() {
  const clones = await instance.detectInFiles(['example.js'])
  console.log(clones)

  await getStoreManager().close(); // Temporary Solution

}
detect().then(detect)

@kucherenko
Copy link
Owner

Hi, I'm back.
I hope you still interesting in the jscpd.

Currently I'm going to release new version of jscpd@3.3.0 and test functionality with jscpd@3.3.0-rc.3.

So, jscpd moved to monorepo and API changed.

For run cli version use following code:

import {IClone} from '@jscpd/core';
import {jscpd} from 'jscpd';
 
const clones: Promise<IClone[]> = jscpd(process.argv);
 
(async () => {
    const clones: IClone[] = await jscpd(['', '', '/path/to/file/or/folder', '-m', 'weak', '--silent']);
})();

If you are going to detect clones in file system you can use @jscpd/finder for make a powerful detector. In case of detect clones in browser or not node.js environment you can build you own solution base on @jscpd/core.

the functionality still in RC, it would be great if you can provide any feedback regarding the functionality and new API. I will release soon to main branch and close the issue.

More info located here - jscpd monorepo

@kucherenko kucherenko added bug An issue contains information about wrong behaviour in progress labels Jun 14, 2020
@kucherenko
Copy link
Owner

fixed in jscpd@3.3.0-rc.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue contains information about wrong behaviour in progress
Projects
None yet
Development

No branches or pull requests

3 participants