Skip to content

Commit

Permalink
chore: enable eslint-plugin-jsdoc internally (#8145)
Browse files Browse the repository at this point in the history
* chore: enable eslint-plugin-jsdoc internally

* Disable jsdoc/check-param-names, with link to issue

* Sort jsdoc rule disables and add links to issues

* Switch typescript-estree-import.ts to line comments
  • Loading branch information
JoshuaKGoldberg authored Jan 7, 2024
1 parent 958feca commit 1ee3087
Show file tree
Hide file tree
Showing 34 changed files with 135 additions and 75 deletions.
22 changes: 21 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module.exports = {
'eslint-plugin',
'import',
'jest',
'jsdoc',
'simple-import-sort',
'unicorn',
],
Expand All @@ -20,6 +21,7 @@ module.exports = {
extends: [
'eslint:recommended',
'plugin:eslint-plugin/recommended',
'plugin:jsdoc/recommended-typescript-error',
'plugin:@typescript-eslint/strict-type-checked',
'plugin:@typescript-eslint/stylistic-type-checked',
],
Expand Down Expand Up @@ -140,6 +142,7 @@ module.exports = {
'error',
{ commentPattern: '.*intentional fallthrough.*' },
],
'one-var': ['error', 'never'],

//
// eslint-plugin-eslint-comment
Expand Down Expand Up @@ -215,7 +218,24 @@ module.exports = {
// enforce a sort order across the codebase
'simple-import-sort/imports': 'error',

'one-var': ['error', 'never'],
//
// eslint-plugin-jsdoc
//

// We often use @remarks or other ad-hoc tag names
'jsdoc/check-tag-names': 'off',
// https://github.com/gajus/eslint-plugin-jsdoc/issues/1169
'jsdoc/check-param-names': 'off',
// https://github.com/gajus/eslint-plugin-jsdoc/issues/1175
'jsdoc/require-jsdoc': 'off',
'jsdoc/require-param': 'off',
'jsdoc/require-returns': 'off',
'jsdoc/require-yields': 'off',
'jsdoc/tag-lines': 'off',

//
// eslint-plugin-unicorn
//

'unicorn/no-typeof-undefined': 'error',
},
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"eslint-plugin-eslint-plugin": "^5.1.0",
"eslint-plugin-import": "^2.27.5",
"eslint-plugin-jest": "^27.2.2",
"eslint-plugin-jsdoc": "^46.9.1",
"eslint-plugin-jsx-a11y": "^6.7.1",
"eslint-plugin-react": "^7.32.2",
"eslint-plugin-react-hooks": "^4.6.0",
Expand Down
26 changes: 12 additions & 14 deletions packages/ast-spec/tests/util/parsers/typescript-estree-import.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
/**
* Nx is picking up on the fact that we technically have a circular dependency between ast-spec
* and typescript-estree.
*
* This circular dependency only occurs in the tests/ for ast-spec and not in the main package source.
*
* We could therefore solve this by separating the ast-spec tests out into their own package, but the
* other option is to get Nx to turn a blind eye to the circular dependency by removing
* @typescript-eslint/typescript-estree as an explicit devDependency in the package.json and just doing an import here.
*
* This file is ignored via a root `.nxignore`
*
* This should be the only place in the package that we import from typescript-estree.
*/
// Nx is picking up on the fact that we technically have a circular dependency between ast-spec
// and typescript-estree.
//
// This circular dependency only occurs in the tests/ for ast-spec and not in the main package source.
//
// We could therefore solve this by separating the ast-spec tests out into their own package, but the
// other option is to get Nx to turn a blind eye to the circular dependency by removing
// @typescript-eslint/typescript-estree as an explicit devDependency in the package.json and just doing an import here.
//
// This file is ignored via a root `.nxignore`
//
// This should be the only place in the package that we import from typescript-estree.

// eslint-disable-next-line no-restricted-imports -- the only safe and valid import from typescript-estree in this package
export { parse, TSError } from '@typescript-eslint/typescript-estree';
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export default createRule({

/**
* Check the body for overload methods.
* @param {ASTNode} node the body to be inspected.
* @param node the body to be inspected.
*/
function checkBodyForOverloadMethods(node: RuleNode): void {
const members = getMembers(node);
Expand Down
3 changes: 2 additions & 1 deletion packages/eslint-plugin/src/rules/consistent-type-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ export default createRule<Options, MessageIds>({
report.unusedSpecifiers.length === 0 &&
report.node.importKind !== 'type'
) {
/** checks if import has type assertions
/**
* checks if import has type assertions
* ```
* import * as type from 'mod' assert { type: 'json' };
* ```
Expand Down
3 changes: 1 addition & 2 deletions packages/eslint-plugin/src/rules/func-call-spacing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ export default createRule<Options, MessageIds>({

/**
* Check if open space is present in a function name
* @param {ASTNode} node node to evaluate
* @returns {void}
* @param node node to evaluate
* @private
*/
function checkSpacing(
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/rules/member-delimiter-style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ export default createRule<Options, MessageIds>({

/**
* Check the member separator being used matches the delimiter.
* @param {ASTNode} node the node to be evaluated.
* @param node the node to be evaluated.
*/
function checkMemberSeparatorStyle(
node: TSESTree.TSInterfaceBody | TSESTree.TSTypeLiteral,
Expand Down
3 changes: 1 addition & 2 deletions packages/eslint-plugin/src/rules/member-ordering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,6 @@ function getMemberRawName(
* Gets the member name based on the member type.
*
* @param node the node to be evaluated.
* @param sourceCode
*/
function getMemberName(
node: Member,
Expand Down Expand Up @@ -801,7 +800,7 @@ export default createRule<Options, MessageIds>({
* Checks if the members are alphabetically sorted.
*
* @param members Members to be validated.
* @param caseSensitive indicates if the alpha ordering is case sensitive or not.
* @param order What order the members should be sorted in.
*
* @return True if all members are correctly sorted.
*/
Expand Down
1 change: 0 additions & 1 deletion packages/eslint-plugin/src/rules/no-loop-func.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export default createRule<Options, MessageIds>({
* - has any references which refers to an unsafe variable.
*
* @param node The AST node to check.
* @returns Whether or not the node is within a loop.
*/
function checkForLoops(
node:
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/rules/no-shadow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ export default createRule<Options, MessageIds>({

/**
* Checks the current context for shadowed variables.
* @param {Scope} scope Fixme
* @param scope Fixme
*/
function checkForShadows(scope: TSESLint.Scope.Scope): void {
// ignore global augmentation
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,8 @@ export default createRule<Options, MessageId>({
* NOTE: It's also unnecessary if the types that don't overlap at all
* but that case is handled by the Typescript compiler itself.
* Known exceptions:
* * https://github.com/microsoft/TypeScript/issues/32627
* * https://github.com/microsoft/TypeScript/issues/37160 (handled)
* - https://github.com/microsoft/TypeScript/issues/32627
* - https://github.com/microsoft/TypeScript/issues/37160 (handled)
*/
const BOOL_OPERATORS = new Set([
'<',
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/rules/no-unused-vars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ export default createRule<Options, MessageIds>({
function collectUnusedVariables(): TSESLint.Scope.Variable[] {
/**
* Checks whether a node is a sibling of the rest property or not.
* @param {ASTNode} node a node to check
* @returns {boolean} True if the node is a sibling of the rest property, otherwise false.
* @param node a node to check
* @returns True if the node is a sibling of the rest property, otherwise false.
*/
function hasRestSibling(node: TSESTree.Node): boolean {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ function isCJSRequire(node: TSESTree.Node): boolean {
/**
* Checks whether the given node is a block-like statement.
* This checks the last token of the node is the closing brace of a block.
* @param sourceCode The source code to get tokens.
* @param node The node to check.
* @param sourceCode The source code to get tokens.
* @returns `true` if the node is a block-like statement.
* @private
*/
Expand Down Expand Up @@ -323,8 +323,8 @@ function isExpression(
*
* foo()
* ;[1, 2, 3].forEach(bar)
* @param sourceCode The source code to get tokens.
* @param node The node to get.
* @param sourceCode The source code to get tokens.
* @returns The actual last token.
* @private
*/
Expand Down
1 change: 0 additions & 1 deletion packages/eslint-plugin/src/rules/prefer-function-type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ export default createRule({
/**
* @param member The TypeElement being checked
* @param node The parent of member being checked
* @param tsThisTypes
*/
function checkMember(
member: TSESTree.TypeElement,
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/rules/prefer-regexp-exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ export default createRule({

/**
* Check if a given node type is a string.
* @param node The node type to check.
* @param type The node type to check.
*/
function isStringType(type: ts.Type): boolean {
return getTypeName(checker, type) === 'string';
}

/**
* Check if a given node type is a RegExp.
* @param node The node type to check.
* @param type The node type to check.
*/
function isRegExpType(type: ts.Type): boolean {
return getTypeName(checker, type) === 'RegExp';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ export default createRule({
/**
* Check if a given node is a `Literal` node that is a character.
* @param node The node to check.
* @param kind The method name to get a character.
*/
function isCharacter(node: TSESTree.Node): node is TSESTree.Literal {
const evaluated = getStaticValue(node, globalScope);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export default createRule<Options, MessageIds>({

/**
* Check if a given node is an array which all elements are string.
* @param node
*/
function isStringArrayNode(node: TSESTree.Expression): boolean {
const type = services.getTypeAtLocation(node);
Expand Down
10 changes: 4 additions & 6 deletions packages/eslint-plugin/src/rules/space-before-function-paren.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ export default createRule<Options, MessageIds>({

/**
* Determines whether a function has a name.
* @param {ASTNode} node The function node.
* @returns {boolean} Whether the function has a name.
* @param node The function node.
* @returns Whether the function has a name.
*/
function isNamedFunction(
node:
Expand All @@ -97,8 +97,7 @@ export default createRule<Options, MessageIds>({

/**
* Gets the config for a given function
* @param {ASTNode} node The function node
* @returns {string} "always", "never", or "ignore"
* @param node The function node
*/
function getConfigForFunction(
node:
Expand Down Expand Up @@ -129,8 +128,7 @@ export default createRule<Options, MessageIds>({

/**
* Checks the parens of a function node
* @param {ASTNode} node A function node
* @returns {void}
* @param node A function node
*/
function checkFunction(
node:
Expand Down
8 changes: 4 additions & 4 deletions packages/eslint-plugin/src/util/astUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ export * from '@typescript-eslint/utils/ast-utils';
// Could be export { getNameLocationInGlobalDirectiveComment } from 'eslint/lib/rules/utils/ast-utils'
/**
* Get the `loc` object of a given name in a `/*globals` directive comment.
* @param {SourceCode} sourceCode The source code to convert index to loc.
* @param {Comment} comment The `/*globals` directive comment which include the name.
* @param {string} name The name to find.
* @returns {SourceLocation} The `loc` object.
* @param sourceCode The source code to convert index to loc.
* @param comment The `/*globals` directive comment which include the name.
* @param name The name to find.
* @returns The `loc` object.
*/
export function getNameLocationInGlobalDirectiveComment(
sourceCode: TSESLint.SourceCode,
Expand Down
1 change: 0 additions & 1 deletion packages/eslint-plugin/src/util/collectUnusedVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,6 @@ const MERGABLE_TYPES = new Set([
/**
* Determine if the variable is directly exported
* @param variable the variable to check
* @param target the type of node that is expected to be exported
*/
function isMergableExported(variable: TSESLint.Scope.Variable): boolean {
// If all of the merged things are of the same type, TS will error if not all of them are exported - so we only need to find one
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,7 @@ function test<T>(a: T) {
}
`,

/**
* Predicate functions
**/
// Predicate functions
`
// with literal arrow function
[0, 1, 2].filter(x => x);
Expand Down
2 changes: 1 addition & 1 deletion packages/parser/tests/tools/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function createSnapshotTestBlock(
config = Object.assign({}, defaultConfig, config);

/**
* @returns {Object} the AST object
* @returns the AST object
*/
function parse(): TSESTree.Program {
const ast = parser.parseForESLint(code, config).ast;
Expand Down
6 changes: 3 additions & 3 deletions packages/rule-tester/src/utils/SourceCodeFixer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function compareMessagesByLocation(a: LintMessage, b: LintMessage): number {
* smart about the fixes and won't apply fixes over the same area in the text.
* @param sourceText The text to apply the changes to.
* @param messages The array of messages reported by ESLint.
* @returns {Object} An object containing the fixed text and any unfixed messages.
* @returns An object containing the fixed text and any unfixed messages.
*/
export function applyFixes(
sourceText: string,
Expand All @@ -54,8 +54,8 @@ export function applyFixes(

/**
* Try to use the 'fix' from a problem.
* @param {Message} problem The message object to apply fixes from
* @returns {boolean} Whether fix was successfully applied
* @param problem The message object to apply fixes from
* @returns Whether fix was successfully applied
*/
function attemptFix(problem: LintMessageWithFix): boolean {
const fix = problem.fix;
Expand Down
2 changes: 1 addition & 1 deletion packages/rule-tester/src/utils/config-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function validateRuleSchema(
* Validates a rule's options against its schema.
* @param rule The rule that the config is being validated for
* @param ruleId The rule's unique name.
* @param {Array|number} options The given options for the rule.
* @param options The given options for the rule.
* @param source The name of the configuration source to report in any errors. If null or undefined,
* no source is prepended to the message.
* @throws {Error} Upon any bad rule configuration.
Expand Down
3 changes: 0 additions & 3 deletions packages/scope-manager/src/ScopeManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ class ScopeManager {
public readonly declaredVariables: WeakMap<TSESTree.Node, Variable[]>;
/**
* The root scope
* @public
*/
public globalScope: GlobalScope | null;
public readonly nodeToScope: WeakMap<TSESTree.Node, Scope[]>;
Expand Down Expand Up @@ -93,7 +92,6 @@ class ScopeManager {
* Get the variables that a given AST node defines. The gotten variables' `def[].node`/`def[].parent` property is the node.
* If the node does not define any variable, this returns an empty array.
* @param node An AST node to get their variables.
* @public
*/
public getDeclaredVariables(node: TSESTree.Node): Variable[] {
return this.declaredVariables.get(node) ?? [];
Expand All @@ -106,7 +104,6 @@ class ScopeManager {
* @param node An AST node to get their scope.
* @param inner If the node has multiple scopes, this returns the outermost scope normally.
* If `inner` is `true` then this returns the innermost scope.
* @public
*/
public acquire(node: TSESTree.Node, inner = false): Scope | null {
function predicate(testScope: Scope): boolean {
Expand Down
2 changes: 1 addition & 1 deletion packages/scope-manager/src/referencer/VisitorBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ abstract class VisitorBase {
/**
* Default method for visiting children.
* @param node the node whose children should be visited
* @param exclude a list of keys to not visit
* @param excludeArr a list of keys to not visit
*/
visitChildren<T extends TSESTree.Node>(
node: T | null | undefined,
Expand Down
Loading

0 comments on commit 1ee3087

Please sign in to comment.