codeclimate-duplication
codeclimate-duplication copied to clipboard
React's propTypes often marked as duplicates
Hi - a common source of false-positives in our CC duplication results comes from React's propType structures. For example:
propTypes: {
playBtnState: React.PropTypes.string.isRequired,
height: React.PropTypes.number.isRequired,
drawBack: React.PropTypes.bool
},
is a 'duplicate' of:
propTypes: {
clipStore: React.PropTypes.object.isRequired,
firstClipId: React.PropTypes.number.isRequired,
includeDetails: React.PropTypes.bool
},
Can the engine be improved to avoid this, or is there anything we can do in our codebase that would help? Excluding each fingerprint on a case-by-case basis is getting tiresome.
@jdelStrother Thank you for the feedback! We are working to improve our Duplication engine's analysis of JavaScript and make results more savvy.
I'm going to tag this issue as Enhancement
and add its consideration to our roadmap.
In the meantime, one workaround might be to to experiment with upping the mass
threshold for JavaScript duplication in your .codeclimate.yml
config settings:
engines:
duplication:
enabled: true
config:
languages:
javascript:
mass_threshold: 50
Our default for JavaScript is 40. Increasing the threshold should make the detection of duplication less sensitive.
Very similar situation occurs when checking Ember imports:
import Ember from 'ember';
import OnboardingButton from 'app/mixins/onboarding-button';
const { Component } = Ember;
is a duplication of something like this ie.:
import Ember from 'ember';
import UnauthenticatedRouteMixin from 'ember-simple-auth/mixins/unauthenticated-route-mixin';
const { Route } = Ember;
export default Route.extend(UnauthenticatedRouteMixin);
This affects overall score a lot :(
@netes @jdelStrother Sorry for the frustration here. Right now the duplication algorithm effectively compares syntax nodes based on both what kind of expression they represent, as well as the contents of those expressions. When the kinds of the expressions are the same, but the contents vary (which looks to be the case here), the engine considers that "similar" duplication to distinguish it from "identical" duplication.
The detection of "similar" duplication like your case is one that, while it can turn up helpful results in many cases, is unfortunately also prone to these kinds of false-positives that are easy to distinguish as a human but not easy to make a decision about within an algorithm. We are continuing to investigate ways of improving the algorithm for these kinds of cases, but need to be careful that in avoiding false-positives we don't swing too far and stop returning useful instances of similar code. So for the time being we don't consider this behavior a bug since this is the expected behavior of Flay, the underlying tool this engine uses for detecting duplication.
There are several workarounds available to you in the meantime:
-
You can ignore particular issues using their fingerprints. This is a good approach if these kinds of reports are an infrequent annoyance for you.
-
As Ashley noted above, you can tune the mass threshold of duplicate code that the engine will report to suite your preferences. This will effect both similar & identical issues. (Ruby's default mass threshold is 40.)
engines: duplication: enabled: true config: javascript: mass_threshold: 50
-
If you would prefer not to see reports of similar code at all, you can disable that check entirely. If you do this, the engine will only report instances of code it sees as identical.
engines: duplication: enabled: true checks: Similar code: enabled: false
@netes If you use any of these workarounds to exclude these issues, they will no longer affect your repo's grade.
See #190 for my proposed solution to this.
This ticket is a bit stale, I just want to note that the work from #190 is now part of the stable
channel, and users should still use that instead of beta
.
We are not counting import
s as duplication by default. Excluding propTypes:
declarations is not curently being done by default, but for users who are still seeing that happen frequently you can configure your own pattens.
Sorry to bring back a stale issue but I'm running into this with
constructor(props){
super(props)
being marked as duplicate and I can't fathom how to filter this out.
It looks like assignments were recently made to team members, but this is something that makes the code duplication rule very difficult to manage in React projects. Really excited to see this feature come in.
To those who landed here and saw @wfleming's suggestion about configuring filter patterns but didn't want to do the legwork of running dump_ast
(like I did), this worked for me in terms of fixing propTypes
defined like this:
Component.propTypes = {
foo: PropTypes.string
}
The following configuration works:
plugins:
duplication:
config:
languages:
javascript:
filters:
- "s(:property, s(:Identifier, :propTypes))"
That configuration is not working for me. I get it working using:
version: "2"
plugins:
duplication:
config:
languages:
javascript:
filters:
- "(object (Identifier PropTypes))"
This also works for me:
version: "2"
plugins:
duplication:
config:
languages:
javascript:
filters:
- "(property (Identifier propTypes))"
I changed it based on this quote:
The internal representation (which is ruby) is different from the pattern language (which is lisp-like), so first we need to convert s(: to ( and remove all commas and colons:
from: https://github.com/codeclimate/codeclimate-duplication
But is still catching repetitions on things like PropTypes.func
or PropTypes.shape
.
I'm testing it using:
/opt/codeclimate/bin/codeclimate analyze --dev -e duplication -f text <path>
I set dump_ast: true
on .codeclimate.yml
.
Thare is no PropType
or propType
on generate AST. But still have shape
, number
, etc.
Facing similar kind of issue, in React js as well. While calling a util component in 2 different components file with different parameter it detects it as similar code.