Skip to content

Commit

Permalink
[New] sort-prop-types: give errors on TS types
Browse files Browse the repository at this point in the history
  • Loading branch information
akulsr0 authored and ljharb committed Aug 9, 2023
1 parent 378de4c commit cab612f
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 4 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ class Component extends React.Component {
"ignoreCase": <boolean>,
"requiredFirst": <boolean>,
"sortShapeProp": <boolean>,
"noSortAlphabetically": <boolean>
"noSortAlphabetically": <boolean>,
"checkTypes": <boolean>

This comment has been minimized.

Copy link
@maksnester

maksnester Feb 14, 2024

Hello there, was it ever released?

I was misled by the documentation mentioning this option and saying that TS types are supported. While in reality, it's impossible to use this option yet.

Using "eslint-plugin-react": "7.33.2",

cc @akulsr0

This comment has been minimized.

Copy link
@ljharb

ljharb Feb 14, 2024

Member

No, it hasn’t been released yet.

Everywhere on github, you need to check the docs under the release tag you have installed, and not on the default branch.

}]
...
```
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
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ module.exports = {
sortShapeProp: {
type: 'boolean',
},
checkTypes: {
type: 'boolean',
},
},
additionalProperties: false,
}],
Expand All @@ -64,8 +67,14 @@ module.exports = {
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 @@ module.exports = {
}
}

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 @@ module.exports = {
}
});
},
};
}, 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) {
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,
} : null);
},
};
100 changes: 99 additions & 1 deletion tests/lib/rules/sort-prop-types.js
Original file line number Diff line number Diff line change
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,
},
],
}
)),
});

0 comments on commit cab612f

Please sign in to comment.