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
feat(eslint-plugin): [require-array-sort-compare] add ignoreStringArrays
option
#1972
Conversation
Thanks for the PR, @sosukesuzuki! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! |
Codecov Report
@@ Coverage Diff @@
## master #1972 +/- ##
==========================================
- Coverage 93.45% 93.44% -0.02%
==========================================
Files 171 171
Lines 7813 7821 +8
Branches 2227 2229 +2
==========================================
+ Hits 7302 7308 +6
- Misses 244 245 +1
- Partials 267 268 +1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your work!
a few comments.
function isStringArrayNode(node: TSESTree.ArrayExpression): boolean { | ||
return node.elements.every(element => { | ||
const type = checker.getTypeAtLocation( | ||
service.esTreeNodeToTSNodeMap.get(element), | ||
); | ||
return util.getTypeName(checker, type) === 'string'; | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this only works for array expressions that have sort
directly called on them: ['a'].sort()
A better solution would be to check the type of the node
instead, and extract the type of the generic parameters.
All array types (string[]
and Array<string>
) are treated as Array<string>
in code.
All tuple types ([a, b, c]
) are treated as Tuple<T1, T2, ...>
(a special variadic type).
const type = checker.getTypeAtLocation(
service.esTreeNodeToTSNodeMap.get(node),
);
if (checker.isArrayType(type) || checker.isTupleType(type)) {
const typeArgs = checker.getTypeArguments(type);
return checker.getTypeArguments(type).every(
arg => util.getTypeName(checker, arg) === 'string',
);
}
return false;
Thank you for your review. I fixed all reviewed points! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks for this!
@sosukesuzuki Thank you for adding this! |
Fixes #1963