codeclimate / codeclimate-duplication

Code Climate engine for code duplication analysis
http://codeclimate.com
MIT License
113 stars 24 forks source link

Detects "Identical blocks of code" that are in no way identical #290

Open pitaj opened 6 years ago

pitaj commented 6 years ago

https://codeclimate.com/github/benchpressjs/benchpressjs/issues

If you scroll down to the two Identical blocks of code found in 2 locations. Consider refactoring. issues, you will see that it detects

  return Promise.try(() => {
    Benchpress.cache[template] = Benchpress.cache[template] || load(template);
    return Benchpress.cache[template];
  }).then((templateFunction) => {
    if (block) {

and

  return Promise.try(() => {
    const cached = compileRenderCache.get(hash);
    if (cached) {
      compileRenderCache.ttl(hash);
      return cached;

as identical when the only thing even similar about them is Promise.try(() => {.

This is super annoying. Is it some kind of diffing or hashing bug? Should I report it on the flay repository?

wfleming commented 6 years ago

Thanks for the report @pitaj, and sorry for the issue.

I'm able to reproduce this, but I'm sorry to say I don't have a fix yet. With some debugging output using the dump_ast config option I was able to see that the engine seems to be incorrectly assigning the AST for the first snippet to both instances of the code. Flay, which this engine is built on, hashes ASTs internally for tracking, so right now I suspect there's a hash conflict here, but I haven't identified exactly where yet. We're still looking into this internally, just wanted you to know it was being investigated. Thanks.

pitaj commented 6 years ago

Thanks for looking into it.

zenspider commented 6 years ago

Just quickly glancing at it... I'd bet it is trying to report the then lambdas, which are identical. I don't know the JS AST so I don't know how it is structured off the top of my head, but I'd bet that the file/line info is screwed up at some level.

zenspider commented 6 years ago

Anything?