Skip to content

Commit

Permalink
Rename jsx-sort-prop-types to sort-prop-types
Browse files Browse the repository at this point in the history
This rule has nothing to do with JSX, so it doesn't make sense for "jsx"
to appear in its name.

To prevent breaking this rule for people, I have set up
jsx-sort-prop-types as an alias with a deprecation warning. Eventually,
we should remove it altogether. For the deprecation warning, I borrowed
code from jsx-quotes.

While I was at it, I corrected some minor wording issues in the
documentation for this rule (mostly using "propTypes" instead of
"propsTypes").

Fixes #87.
  • Loading branch information
lencioni committed Feb 8, 2016
1 parent b2661aa commit 02116f9
Show file tree
Hide file tree
Showing 6 changed files with 777 additions and 130 deletions.
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Enforce propTypes declarations alphabetical sorting (jsx-sort-prop-types)
# Enforce propTypes declarations alphabetical sorting (sort-prop-types)

Some developers prefer to sort propsTypes declarations alphabetically to be able to find necessary declaration easier at the later time. Others feel that it adds complexity and becomes burden to maintain.
Some developers prefer to sort propTypes declarations alphabetically to be able to find necessary declaration easier at the later time. Others feel that it adds complexity and becomes burden to maintain.

## Rule Details

This rule checks all JSX components and verifies that all propsTypes declarations are sorted alphabetically.
This rule checks all components and verifies that all propTypes declarations are sorted alphabetically.
The default configuration of the rule is case-sensitive.
This rule is off by default.

Expand Down Expand Up @@ -78,7 +78,7 @@ class Component extends React.Component {

```js
...
"jsx-sort-prop-types": [<enabled>, {
"sort-prop-types": [<enabled>, {
"callbacksLast": <boolean>,
"ignoreCase": <boolean>,
"requiredFirst": <boolean>,
Expand All @@ -92,7 +92,7 @@ When `true` the rule ignores the case-sensitivity of the declarations order.

### `callbacksLast`

When `true`, prop types for props beginning with "on" must be listed after all other props:
When `true`, propTypes for props beginning with "on" must be listed after all other props:

```js
var Component = React.createClass({
Expand Down
2 changes: 2 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module.exports = {
'jsx-curly-spacing': require('./lib/rules/jsx-curly-spacing'),
'jsx-equals-spacing': require('./lib/rules/jsx-equals-spacing'),
'jsx-sort-props': require('./lib/rules/jsx-sort-props'),
'sort-prop-types': require('./lib/rules/sort-prop-types'),
'jsx-sort-prop-types': require('./lib/rules/jsx-sort-prop-types'),
'jsx-boolean-value': require('./lib/rules/jsx-boolean-value'),
'sort-comp': require('./lib/rules/sort-comp'),
Expand Down Expand Up @@ -66,6 +67,7 @@ module.exports = {
'jsx-curly-spacing': 0,
'jsx-equals-spacing': 0,
'jsx-sort-props': 0,
'sort-prop-types': 0,
'jsx-sort-prop-types': 0,
'jsx-boolean-value': 0,
'sort-comp': 0,
Expand Down
138 changes: 15 additions & 123 deletions lib/rules/jsx-sort-prop-types.js
Original file line number Diff line number Diff line change
@@ -1,140 +1,32 @@
/**
* @fileoverview Enforce propTypes declarations alphabetical sorting
* @deprecated
*/
'use strict';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = function(context) {

var configuration = context.options[0] || {};
var requiredFirst = configuration.requiredFirst || false;
var callbacksLast = configuration.callbacksLast || false;
var ignoreCase = configuration.ignoreCase || false;

/**
* Checks if node is `propTypes` declaration
* @param {ASTNode} node The AST node being checked.
* @returns {Boolean} True if node is `propTypes` declaration, false if not.
*/
function isPropTypesDeclaration(node) {

// Special case for class properties
// (babel-eslint does not expose property name so we have to rely on tokens)
if (node.type === 'ClassProperty') {
var tokens = context.getFirstTokens(node, 2);
return (tokens[0] && tokens[0].value === 'propTypes') ||
(tokens[1] && tokens[1].value === 'propTypes');
}

return Boolean(
node &&
node.name === 'propTypes'
);
}

function getKey(node) {
return node.key.type === 'Identifier' ? node.key.name : node.key.value;
}

function getValueName(node) {
return node.value.property && node.value.property.name;
}

function isCallbackPropName(propName) {
return /^on[A-Z]/.test(propName);
}

function isRequiredProp(node) {
return getValueName(node) === 'isRequired';
}

/**
* Checks if propTypes declarations are sorted
* @param {Array} declarations The array of AST nodes being checked.
* @returns {void}
*/
function checkSorted(declarations) {
declarations.reduce(function(prev, curr) {
var prevPropName = getKey(prev);
var currentPropName = getKey(curr);
var previousIsRequired = isRequiredProp(prev);
var currentIsRequired = isRequiredProp(curr);
var previousIsCallback = isCallbackPropName(prevPropName);
var currentIsCallback = isCallbackPropName(currentPropName);

if (ignoreCase) {
prevPropName = prevPropName.toLowerCase();
currentPropName = currentPropName.toLowerCase();
}
var util = require('util');
var sortPropTypes = require('./sort-prop-types');
var isWarnedForDeprecation = false;

if (requiredFirst) {
if (previousIsRequired && !currentIsRequired) {
// Transition between required and non-required. Don't compare for alphabetical.
return curr;
}
if (!previousIsRequired && currentIsRequired) {
// Encountered a non-required prop after a required prop
context.report(curr, 'Required prop types must be listed before all other prop types');
return curr;
}
}

if (callbacksLast) {
if (!previousIsCallback && currentIsCallback) {
// Entering the callback prop section
return curr;
}
if (previousIsCallback && !currentIsCallback) {
// Encountered a non-callback prop after a callback prop
context.report(prev, 'Callback prop types must be listed after all other prop types');
return prev;
}
}

if (currentPropName < prevPropName) {
context.report(curr, 'Prop types declarations should be sorted alphabetically');
return prev;
}

return curr;
}, declarations[0]);
}

return {
ClassProperty: function(node) {
if (isPropTypesDeclaration(node) && node.value && node.value.type === 'ObjectExpression') {
checkSorted(node.value.properties);
}
},

MemberExpression: function(node) {
if (isPropTypesDeclaration(node.property)) {
var right = node.parent.right;
if (right && right.type === 'ObjectExpression') {
checkSorted(right.properties);
}
module.exports = function(context) {
return util._extend(sortPropTypes(context), {
Program: function() {
if (isWarnedForDeprecation || /\=-(f|-format)=/.test(process.argv.join('='))) {
return;
}
},

ObjectExpression: function(node) {
node.properties.forEach(function(property) {
if (!property.key) {
return;
}

if (!isPropTypesDeclaration(property.key)) {
return;
}
if (property.value.type === 'ObjectExpression') {
checkSorted(property.value.properties);
}
});
/* eslint-disable no-console */
console.log('The react/jsx-sort-prop-types rule is deprecated. Please ' +
'use the react/sort-prop-types rule instead.');
/* eslint-enable no-console */
isWarnedForDeprecation = true;
}

};
});
};

module.exports.schema = [{
Expand Down
154 changes: 154 additions & 0 deletions lib/rules/sort-prop-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/**
* @fileoverview Enforce propTypes declarations alphabetical sorting
*/
'use strict';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = function(context) {

var configuration = context.options[0] || {};
var requiredFirst = configuration.requiredFirst || false;
var callbacksLast = configuration.callbacksLast || false;
var ignoreCase = configuration.ignoreCase || false;

/**
* Checks if node is `propTypes` declaration
* @param {ASTNode} node The AST node being checked.
* @returns {Boolean} True if node is `propTypes` declaration, false if not.
*/
function isPropTypesDeclaration(node) {

// Special case for class properties
// (babel-eslint does not expose property name so we have to rely on tokens)
if (node.type === 'ClassProperty') {
var tokens = context.getFirstTokens(node, 2);
return (tokens[0] && tokens[0].value === 'propTypes') ||
(tokens[1] && tokens[1].value === 'propTypes');
}

return Boolean(
node &&
node.name === 'propTypes'
);
}

function getKey(node) {
return node.key.type === 'Identifier' ? node.key.name : node.key.value;
}

function getValueName(node) {
return node.value.property && node.value.property.name;
}

function isCallbackPropName(propName) {
return /^on[A-Z]/.test(propName);
}

function isRequiredProp(node) {
return getValueName(node) === 'isRequired';
}

/**
* Checks if propTypes declarations are sorted
* @param {Array} declarations The array of AST nodes being checked.
* @returns {void}
*/
function checkSorted(declarations) {
declarations.reduce(function(prev, curr) {
var prevPropName = getKey(prev);
var currentPropName = getKey(curr);
var previousIsRequired = isRequiredProp(prev);
var currentIsRequired = isRequiredProp(curr);
var previousIsCallback = isCallbackPropName(prevPropName);
var currentIsCallback = isCallbackPropName(currentPropName);

if (ignoreCase) {
prevPropName = prevPropName.toLowerCase();
currentPropName = currentPropName.toLowerCase();
}

if (requiredFirst) {
if (previousIsRequired && !currentIsRequired) {
// Transition between required and non-required. Don't compare for alphabetical.
return curr;
}
if (!previousIsRequired && currentIsRequired) {
// Encountered a non-required prop after a required prop
context.report(curr, 'Required prop types must be listed before all other prop types');
return curr;
}
}

if (callbacksLast) {
if (!previousIsCallback && currentIsCallback) {
// Entering the callback prop section
return curr;
}
if (previousIsCallback && !currentIsCallback) {
// Encountered a non-callback prop after a callback prop
context.report(prev, 'Callback prop types must be listed after all other prop types');
return prev;
}
}

if (currentPropName < prevPropName) {
context.report(curr, 'Prop types declarations should be sorted alphabetically');
return prev;
}

return curr;
}, declarations[0]);
}

return {
ClassProperty: function(node) {
if (isPropTypesDeclaration(node) && node.value && node.value.type === 'ObjectExpression') {
checkSorted(node.value.properties);
}
},

MemberExpression: function(node) {
if (isPropTypesDeclaration(node.property)) {
var right = node.parent.right;
if (right && right.type === 'ObjectExpression') {
checkSorted(right.properties);
}
}
},

ObjectExpression: function(node) {
node.properties.forEach(function(property) {
if (!property.key) {
return;
}

if (!isPropTypesDeclaration(property.key)) {
return;
}
if (property.value.type === 'ObjectExpression') {
checkSorted(property.value.properties);
}
});
}

};
};

module.exports.schema = [{
type: 'object',
properties: {
requiredFirst: {
type: 'boolean'
},
callbacksLast: {
type: 'boolean'
},
ignoreCase: {
type: 'boolean'
}
},
additionalProperties: false
}];
4 changes: 2 additions & 2 deletions tests/lib/rules/jsx-sort-prop-types.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* @fileoverview Tests for jsx-sort-prop-types
* @fileoverview Tests for sort-prop-types
*/
'use strict';

Expand Down Expand Up @@ -27,7 +27,7 @@ require('babel-eslint');
var ERROR_MESSAGE = 'Prop types declarations should be sorted alphabetically';

var ruleTester = new RuleTester();
ruleTester.run('jsx-sort-prop-types', rule, {
ruleTester.run('sort-prop-types', rule, {

valid: [{
code: [
Expand Down

0 comments on commit 02116f9

Please sign in to comment.