-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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-types-exports] add new rule #8443
Open
StyleShit
wants to merge
46
commits into
typescript-eslint:main
Choose a base branch
from
StyleShit:feat/require-types-exports
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
9a0c28a
feat(eslint-plugin): [require-types-exports] add new rule
StyleShit 7778868
wip
StyleShit 12fce5b
wip
StyleShit d62f86c
lint
StyleShit 0ebebd2
wip
StyleShit 30e9aa9
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit bfee791
spelling...
StyleShit b309b51
wip
StyleShit 6aa6446
wip
StyleShit 892c368
wip
StyleShit 0e8e58f
tuple generic
StyleShit f4018a8
wip
StyleShit 89a8344
wip
StyleShit b2138e3
wip
StyleShit feedefd
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit 1161db0
wip
StyleShit d9875b3
refactor
StyleShit 6338202
make it shorter & more readable
StyleShit 428b2c1
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit 2cb3455
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit 1812e37
fix nested types in functions
StyleShit 4bee779
fix docs
StyleShit 26e7be7
add inferred return type test case
StyleShit e57985a
stupidly check for variable types
StyleShit cbb784c
support default exported variable
StyleShit 2f2dfa4
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit 37a0171
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit 6fb274a
update docs
StyleShit 4672fe1
wip
StyleShit c79b5cb
wip
StyleShit 279055a
wip
StyleShit 7897abf
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit 7082960
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit 2f81933
improve types
StyleShit 0f788d2
improve type reference search
StyleShit 6cec0f5
don't report types from default library
StyleShit 497957a
getTypeName
StyleShit 702d4d0
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit 700ff85
move utils out of the closure
StyleShit 9a155b3
support namespaced types
StyleShit 8d0d000
Merge remote-tracking branch 'typescript-eslint/main' into feat/requi…
StyleShit b65f9c4
fix namespaced imports
StyleShit 078e24a
WIP
StyleShit ed23162
wip
StyleShit ac224eb
fix propertykey tests
StyleShit 417cc91
ReturnType test
StyleShit File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
288 changes: 288 additions & 0 deletions
288
packages/eslint-plugin/src/rules/require-types-exports.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,288 @@ | ||
import type { TSESTree } from '@typescript-eslint/utils'; | ||
import { AST_NODE_TYPES } from '@typescript-eslint/utils'; | ||
|
||
import { createRule } from '../util'; | ||
|
||
type MessageIds = 'requireTypeExport'; | ||
|
||
export default createRule<[], MessageIds>({ | ||
name: 'require-types-exports', | ||
meta: { | ||
type: 'suggestion', | ||
docs: { | ||
recommended: 'strict', | ||
description: | ||
'Require exporting types that are used in exported functions declarations', | ||
}, | ||
messages: { | ||
requireTypeExport: 'Expected type "{{ name }}" to be exported', | ||
}, | ||
schema: [], | ||
}, | ||
defaultOptions: [], | ||
create(context) { | ||
const exportedTypes = new Set<string>(); | ||
const reported = new Set<string>(); | ||
|
||
function visitExportedFunctionDeclaration( | ||
node: TSESTree.ExportNamedDeclaration & { | ||
declaration: TSESTree.FunctionDeclaration | TSESTree.TSDeclareFunction; | ||
}, | ||
): void { | ||
checkFunctionParamsTypes(node.declaration); | ||
checkFunctionReturnType(node.declaration); | ||
} | ||
|
||
function visitExportedVariableDeclaration( | ||
node: TSESTree.ExportNamedDeclaration & { | ||
declaration: TSESTree.VariableDeclaration; | ||
}, | ||
): void { | ||
node.declaration.declarations.forEach(declaration => { | ||
if (declaration.init?.type === AST_NODE_TYPES.ArrowFunctionExpression) { | ||
checkFunctionParamsTypes(declaration.init); | ||
checkFunctionReturnType(declaration.init); | ||
} | ||
}); | ||
} | ||
|
||
function checkFunctionParamsTypes( | ||
node: | ||
| TSESTree.FunctionDeclaration | ||
| TSESTree.TSDeclareFunction | ||
| TSESTree.ArrowFunctionExpression, | ||
): void { | ||
node.params.forEach(param => { | ||
getParamTypesNodes(param).forEach(paramTypeNode => { | ||
const name = getTypeName(paramTypeNode); | ||
|
||
if (!name) { | ||
// TODO: Report on the whole function? | ||
return; | ||
} | ||
|
||
const isExported = exportedTypes.has(name); | ||
const isReported = reported.has(name); | ||
|
||
if (isExported || isReported) { | ||
return; | ||
} | ||
|
||
context.report({ | ||
node: paramTypeNode, | ||
messageId: 'requireTypeExport', | ||
data: { | ||
name, | ||
}, | ||
}); | ||
|
||
reported.add(name); | ||
}); | ||
}); | ||
} | ||
|
||
function checkFunctionReturnType( | ||
node: | ||
| TSESTree.FunctionDeclaration | ||
| TSESTree.TSDeclareFunction | ||
| TSESTree.ArrowFunctionExpression, | ||
): void { | ||
const returnTypeNode = node.returnType; | ||
|
||
if (!returnTypeNode) { | ||
return; | ||
} | ||
|
||
getReturnTypesNodes(returnTypeNode).forEach(returnTypeNode => { | ||
const name = getTypeName(returnTypeNode); | ||
|
||
if (!name) { | ||
return; | ||
} | ||
|
||
const isExported = exportedTypes.has(name); | ||
const isReported = reported.has(name); | ||
|
||
if (isExported || isReported) { | ||
return; | ||
} | ||
|
||
context.report({ | ||
node: returnTypeNode, | ||
messageId: 'requireTypeExport', | ||
data: { | ||
name, | ||
}, | ||
}); | ||
|
||
reported.add(name); | ||
}); | ||
} | ||
|
||
function getParamTypesNodes( | ||
param: TSESTree.Parameter, | ||
): TSESTree.TSTypeReference[] { | ||
// Single type | ||
if ( | ||
param.type === AST_NODE_TYPES.Identifier && | ||
param.typeAnnotation?.typeAnnotation.type === | ||
AST_NODE_TYPES.TSTypeReference | ||
) { | ||
return [param.typeAnnotation.typeAnnotation]; | ||
} | ||
|
||
// Union or intersection | ||
if ( | ||
param.type === AST_NODE_TYPES.Identifier && | ||
(param.typeAnnotation?.typeAnnotation.type === | ||
AST_NODE_TYPES.TSUnionType || | ||
param.typeAnnotation?.typeAnnotation.type === | ||
AST_NODE_TYPES.TSIntersectionType) | ||
) { | ||
return param.typeAnnotation.typeAnnotation.types.filter( | ||
type => type.type === AST_NODE_TYPES.TSTypeReference, | ||
) as TSESTree.TSTypeReference[]; | ||
} | ||
|
||
// Tuple | ||
if ( | ||
param.type === AST_NODE_TYPES.ArrayPattern && | ||
param.typeAnnotation?.typeAnnotation.type === AST_NODE_TYPES.TSTupleType | ||
) { | ||
return param.typeAnnotation.typeAnnotation.elementTypes.filter( | ||
type => type.type === AST_NODE_TYPES.TSTypeReference, | ||
) as TSESTree.TSTypeReference[]; | ||
} | ||
|
||
// Inline object | ||
if ( | ||
param.type === AST_NODE_TYPES.ObjectPattern && | ||
param.typeAnnotation?.typeAnnotation.type === | ||
AST_NODE_TYPES.TSTypeLiteral | ||
) { | ||
return param.typeAnnotation.typeAnnotation.members.reduce< | ||
TSESTree.TSTypeReference[] | ||
>((acc, member) => { | ||
if ( | ||
member.type === AST_NODE_TYPES.TSPropertySignature && | ||
member.typeAnnotation?.typeAnnotation.type === | ||
AST_NODE_TYPES.TSTypeReference | ||
) { | ||
acc.push(member.typeAnnotation.typeAnnotation); | ||
} | ||
|
||
return acc; | ||
}, []); | ||
} | ||
|
||
// Rest params | ||
if ( | ||
param.type === AST_NODE_TYPES.RestElement && | ||
param.typeAnnotation?.typeAnnotation.type === | ||
AST_NODE_TYPES.TSArrayType && | ||
param.typeAnnotation.typeAnnotation.elementType.type === | ||
AST_NODE_TYPES.TSTypeReference | ||
) { | ||
return [param.typeAnnotation.typeAnnotation.elementType]; | ||
} | ||
|
||
// Default value assignment | ||
if ( | ||
param.type === AST_NODE_TYPES.AssignmentPattern && | ||
param.left.typeAnnotation?.typeAnnotation.type === | ||
AST_NODE_TYPES.TSTypeReference | ||
) { | ||
return [param.left.typeAnnotation.typeAnnotation]; | ||
} | ||
|
||
return []; | ||
} | ||
|
||
function getReturnTypesNodes( | ||
typeAnnotation: TSESTree.TSTypeAnnotation, | ||
): TSESTree.TSTypeReference[] { | ||
// Single type | ||
if ( | ||
typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSTypeReference | ||
) { | ||
return [typeAnnotation.typeAnnotation]; | ||
} | ||
|
||
// Union or intersection | ||
if ( | ||
typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSUnionType || | ||
typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSIntersectionType | ||
) { | ||
return typeAnnotation.typeAnnotation.types.filter( | ||
type => type.type === AST_NODE_TYPES.TSTypeReference, | ||
) as TSESTree.TSTypeReference[]; | ||
} | ||
|
||
// Tuple | ||
if (typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSTupleType) { | ||
return typeAnnotation.typeAnnotation.elementTypes.filter( | ||
type => type.type === AST_NODE_TYPES.TSTypeReference, | ||
) as TSESTree.TSTypeReference[]; | ||
} | ||
|
||
// Inline object | ||
if (typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSTypeLiteral) { | ||
return typeAnnotation.typeAnnotation.members.reduce< | ||
TSESTree.TSTypeReference[] | ||
>((acc, member) => { | ||
if ( | ||
member.type === AST_NODE_TYPES.TSPropertySignature && | ||
member.typeAnnotation?.typeAnnotation.type === | ||
AST_NODE_TYPES.TSTypeReference | ||
) { | ||
acc.push(member.typeAnnotation.typeAnnotation); | ||
} | ||
|
||
return acc; | ||
}, []); | ||
} | ||
|
||
return []; | ||
} | ||
|
||
function collectExportedTypes(node: TSESTree.Program): void { | ||
node.body.forEach(statement => { | ||
if (statement.type !== AST_NODE_TYPES.ExportNamedDeclaration) { | ||
return; | ||
} | ||
|
||
const { declaration } = statement; | ||
|
||
if ( | ||
declaration?.type === AST_NODE_TYPES.TSTypeAliasDeclaration || | ||
declaration?.type === AST_NODE_TYPES.TSInterfaceDeclaration | ||
) { | ||
exportedTypes.add(declaration.id.name); | ||
|
||
return; | ||
} | ||
}); | ||
} | ||
|
||
function getTypeName(typeReference: TSESTree.TSTypeReference): string { | ||
if (typeReference.typeName.type === AST_NODE_TYPES.Identifier) { | ||
return typeReference.typeName.name; | ||
} | ||
|
||
return ''; | ||
} | ||
|
||
return { | ||
Program: collectExportedTypes, | ||
|
||
'ExportNamedDeclaration[declaration.type="FunctionDeclaration"]': | ||
visitExportedFunctionDeclaration, | ||
|
||
'ExportNamedDeclaration[declaration.type="TSDeclareFunction"]': | ||
visitExportedFunctionDeclaration, | ||
|
||
'ExportNamedDeclaration[declaration.type="VariableDeclaration"]': | ||
visitExportedVariableDeclaration, | ||
}; | ||
}, | ||
}); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
OK
so I got into a rabbit hole here...
to which level should we follow the exported functions?
should we support things like this?
or this?
and to which level should we follow the types?
// (-- imagine the same nesting but with types --)
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.
IMO if this rule is adhered to, there should never be a case where folks want a type that's not exported. IMO that includes nested functions like
(default|functions).obj.func4
: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.
Started working on this
There is this magical
Scope.through
thing that basically saved me hours of work! ✨StyleShit@1812e37
Is there anything similar for variables, before I start going crazy with those infinite conditions again?
To make it clear - I'm looking for something that extracts the type references from this for example:
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.
In addition, is there any utility to run recursively on objects like the one you mentioned above? (
functions.obj.func4()
)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.
To be honest, I'm not super practiced with
@typescript-eslint/scope-manager
myself 😅 and don't have a great answer for you. Sorry. I'd say your best bet is to play around with the APIs and the scopes on the playground for your snippet.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.
I have a stupid idea that works, not sure whether it's acceptable 😂
will probably need a review on that before I use the same approach for the rest:
e57985a
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.
bump?
will be glad to get some directions here, I'm lost...
I have an awful WIP code locally that collects all the type references from the Program, puts them in a Set, and tries to guess the connection between them and the functions' scopes they're related to
(hint: it's ugly and doesn't work 😓)
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.
Heya sorry, was out - when you say "doesn't work", what do you mean?
Re e57985a#diff-c33471d1953c12297755643e4d460779c5a5323f4535a87cba96ff86abeb40d4R140-R142:
You might be able to check
ts.Node
s by referential equality. Like, ifr.identifier === node
, or some similar thing that maybe involves a.parent
.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.
by "not working" I mean "I can't make it guess properly" haha
regarding the referential equality check - I managed to make it work thanks to your direction, thanks!
anyway, I still can't find a way to get the types used in an exported object properly 😓
I know I can get the full type as a string using
typeToString()
, but is there a way to get it as an object of nodes/types or something? because seems like this might help me figure out which types have been usedThere 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.
I think you'll have to do this manually: a function that takes in an AST node, descends through the AST node, and collects all the types (identifiers & object literals). I don't know of any existing utilities to do this. 😞