eslint-plugin-react
eslint-plugin-react copied to clipboard
Use more efficient regex for boolean-prop-naming
The default RegExp for the boolean-prop-naming rule (^(is|has)[A-Z]([A-Za-z0-9]?)+) has an odd syntax that is inefficient and does not test the entire property name.
For example, the following propTypes passes with the current RegExp.
var Hello = createReactClass({
propTypes: {
// This property name has an underscore, but still
// passes as a valid boolean property name.
isGood_Enough: PropTypes.bool
},
render: function() { return <div />; };
});
This PR assigns a new default RegExp (^(is|has)[A-Z][A-Za-z0-9]*$) that is not only more performant but also tests the entire property name.
This should probably be rebased with or merged into #1702.
This would be a breaking change; isGood_Enough is likely intentionally allowed. Separately, performance rarely matters, and the performance of a regex applied in linting - not even a hot code path - also doesn't matter.
If you can improve the regex while keeping what it matches identical, I'd love to merge that!
I understand if you don't want to introduce a breaking change. An even more efficient yet compatible RegExp is ^(is|has)[A-Z]. I've updated the PR to reflect that. Let me know if you want me to squash the commits.
I still disagree that isGood_Enough is intentionally allowed. The original RegExp appears to have meant to check the entire property name and it did not include any underscores. Note that the property name isA_$$$_Maker is also valid with the original RegExp. It seems odd to enforce a camelCase prefix and then allow snake_case for the rest of the property name.
One issue I still see with this improvement is that it makes it less clear what is intended. For example a user might see the lint error Prop name (enabled) doesn't match rule (^(is|has)[A-Z]) and conclude that boolean property names are only allowed to have one uppercase letter after is or has and nothing else. Alternatively, I can set the default RegExp to ^(is|has)[A-Z][A-Za-z0-9]*. This is less efficient than ^(is|has)[A-Z] but is still more efficient than the original and it is easier for humans to understand. However, this can also be misleading since users may think that it checks the entire property name.