Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New] sort-prop-types: give errors on TS types #3615

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,11 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

## Unreleased

### Added
* [`sort-prop-types`]: give errors on TS types ([#3615][] @akulsr0)

[#3615]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3615

## [7.33.2] - 2023.08.15

### Fixed
Expand Down
7 changes: 6 additions & 1 deletion docs/rules/sort-prop-types.md
Expand Up @@ -95,7 +95,8 @@ class Component extends React.Component {
"ignoreCase": <boolean>,
"requiredFirst": <boolean>,
"sortShapeProp": <boolean>,
"noSortAlphabetically": <boolean>
"noSortAlphabetically": <boolean>,
"checkTypes": <boolean>
}]
...
```
Expand Down Expand Up @@ -170,6 +171,10 @@ var Component = createReactClass({
});
```

### `checkTypes`

When `true`, the sorting of prop type definitions are checked.

## When Not To Use It

This rule is a formatting preference and not following it won't negatively affect the quality of your code. If alphabetizing props declarations isn't a part of your coding standards, then you can leave this rule off.
70 changes: 68 additions & 2 deletions lib/rules/sort-prop-types.js
Expand Up @@ -52,6 +52,9 @@
sortShapeProp: {
type: 'boolean',
},
checkTypes: {
type: 'boolean',
},
},
additionalProperties: false,
}],
Expand All @@ -64,8 +67,14 @@
const ignoreCase = configuration.ignoreCase || false;
const noSortAlphabetically = configuration.noSortAlphabetically || false;
const sortShapeProp = configuration.sortShapeProp || false;
const checkTypes = configuration.checkTypes || false;

const typeAnnotations = new Map();

function getKey(node) {
if (node.type === 'ObjectTypeProperty') {
return context.getSourceCode().getFirstToken(node).value;
}
if (node.key && node.key.value) {
return node.key.value;
}
Expand Down Expand Up @@ -215,7 +224,32 @@
}
}

return {
function handleFunctionComponent(node) {
const firstArg = node.params[0].typeAnnotation && node.params[0].typeAnnotation.typeAnnotation;
if (firstArg && firstArg.type === 'TSTypeReference') {
const propType = typeAnnotations.get(firstArg.typeName.name)
&& typeAnnotations.get(firstArg.typeName.name)[0];
if (propType && propType.members) {
checkSorted(propType.members);

Check warning on line 233 in lib/rules/sort-prop-types.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/sort-prop-types.js#L230-L233

Added lines #L230 - L233 were not covered by tests
}
} else if (firstArg && firstArg.type === 'TSTypeLiteral') {
if (firstArg.members) {
checkSorted(firstArg.members);

Check warning on line 237 in lib/rules/sort-prop-types.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/sort-prop-types.js#L236-L237

Added lines #L236 - L237 were not covered by tests
}
} else if (firstArg && firstArg.type === 'GenericTypeAnnotation') {
const propType = typeAnnotations.get(firstArg.id.name)
&& typeAnnotations.get(firstArg.id.name)[0];
if (propType && propType.properties) {
checkSorted(propType.properties);
}
} else if (firstArg && firstArg.type === 'ObjectTypeAnnotation') {
if (firstArg.properties) {
checkSorted(firstArg.properties);
}
}
}

return Object.assign({
CallExpression(node) {
if (!sortShapeProp || !isShapeProp(node) || !(node.arguments && node.arguments[0])) {
return;
Expand Down Expand Up @@ -261,6 +295,38 @@
}
});
},
};
}, checkTypes ? {
TSTypeLiteral(node) {
if (node && node.parent.id) {
const currentNode = [].concat(
typeAnnotations.get(node.parent.id.name) || [],

Check warning on line 302 in lib/rules/sort-prop-types.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/sort-prop-types.js#L299-L302

Added lines #L299 - L302 were not covered by tests
node
);
typeAnnotations.set(node.parent.id.name, currentNode);

Check warning on line 305 in lib/rules/sort-prop-types.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/sort-prop-types.js#L305

Added line #L305 was not covered by tests
}
},

TypeAlias(node) {
if (node.right.type === 'ObjectTypeAnnotation') {
const currentNode = [].concat(
typeAnnotations.get(node.id.name) || [],
node.right
);
typeAnnotations.set(node.id.name, currentNode);
}
},

TSTypeAliasDeclaration(node) {
ljharb marked this conversation as resolved.
Show resolved Hide resolved
if (node.typeAnnotation.type === 'TSTypeLiteral' || node.typeAnnotation.type === 'ObjectTypeAnnotation') {
const currentNode = [].concat(
typeAnnotations.get(node.id.name) || [],

Check warning on line 322 in lib/rules/sort-prop-types.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/sort-prop-types.js#L319-L322

Added lines #L319 - L322 were not covered by tests
node.typeAnnotation
);
typeAnnotations.set(node.id.name, currentNode);

Check warning on line 325 in lib/rules/sort-prop-types.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/sort-prop-types.js#L325

Added line #L325 was not covered by tests
}
},
FunctionDeclaration: handleFunctionComponent,
ArrowFunctionExpression: handleFunctionComponent,
ljharb marked this conversation as resolved.
Show resolved Hide resolved
} : null);
},
};
100 changes: 99 additions & 1 deletion tests/lib/rules/sort-prop-types.js
Expand Up @@ -480,6 +480,19 @@ ruleTester.run('sort-prop-types', rule, {
});
`,
options: [{ callbacksLast: true, noSortAlphabetically: true }],
},
{
code: `
type Props = {
zzz: string;
aaa: string;
}
function Foo(props: Props) {
return null;
}
`,
features: ['types'],
options: [{ checkTypes: false }],
}
)),
invalid: parsers.all([].concat(
Expand Down Expand Up @@ -2250,6 +2263,91 @@ ruleTester.run('sort-prop-types', rule, {
line: 4,
},
],
} : []
} : [],
{
code: `
type Props = {
zzz: string;
aaa: string;
}
function Foo(props: Props) {
return null;
}
`,
output: `
type Props = {
aaa: string;
zzz: string;
}
function Foo(props: Props) {
return null;
}
`,
features: ['types'],
options: [{ checkTypes: true }],
errors: [
{
messageId: 'propsNotSorted',
line: 4,
column: 11,
},
],
},
{
code: `
type Props = {
zzz: string;
aaa: string;
}
const Foo = (props: Props) => {
return null;
}
`,
output: `
type Props = {
aaa: string;
zzz: string;
}
const Foo = (props: Props) => {
return null;
}
`,
features: ['types'],
options: [{ checkTypes: true }],
errors: [
{
messageId: 'propsNotSorted',
line: 4,
column: 11,
},
],
},
{
code: `
const Foo = (props: {
zzz: string,
aaa: string,
}) => {
return null;
}
`,
output: `
const Foo = (props: {
aaa: string,
zzz: string,
}) => {
return null;
}
`,
features: ['types'],
options: [{ checkTypes: true }],
errors: [
{
messageId: 'propsNotSorted',
line: 4,
column: 11,
},
],
}
)),
});