Don't report same line for multiple duplications #37
Conversation
| @@ -26,6 +26,10 @@ def format | |||
| } | |||
| end | |||
|
|
|||
| def report_name | |||
| "#{current_sexp.file}-#{current_sexp.line}" | |||
There was a problem hiding this comment.
Should this include end line as well?
There was a problem hiding this comment.
I was thinking about that myself, and I'm not sure honestly. I don't think there would be any (or very many at the least) instances where the same start line has multiple end-lines.
I'd love to hear what others think about that though.
There was a problem hiding this comment.
Cool - You know more about this so I defer to you on that!
|
LGTM |
This uses Ruby's [`set`](http://ruby-doc.org/stdlib-2.2.3/libdoc/set/rdoc/Set.html) class to keep track of which lines in every file is reported to prevent reporting the same line multiple times. This happens most frequently with JavaScript since the Babel generated ast is verbose and can expand a single line into a large s-expression tree which can have several duplications.
BlakeWilliams
force-pushed
the
bmw-single-line-duplications
branch
from
1892130 to
3db399f
Compare
|
|
||
| result = run_engine(engine_conf).strip | ||
| issues = result.split("\0") | ||
| expect(issues.length).to eq 1 |
There was a problem hiding this comment.
@BlakeWilliams can you explain why this is 1? Is it because we set the mass to 1 for the tests? I wouldn't expect a line to be reported unless there was another instance of it
There was a problem hiding this comment.
Yep, it's because we pass in an extremely low mass threshold so this will always be higher.
Don't report same line for multiple duplications
This uses Ruby's
setclassto keep track of which lines in every file is reported to prevent
reporting the same line multiple times.
This happens most frequently with JavaScript since the Babel generated
ast is verbose and can expand a single line into a large s-expression
tree which can have several duplications.