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

Chore: enable eslint-plugin-jsdoc (refs #11146) #12332

Merged
merged 17 commits into from Sep 29, 2019
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
6 changes: 2 additions & 4 deletions lib/cli-engine/cli-engine.js
Expand Up @@ -85,8 +85,8 @@ const validFixTypes = new Set(["problem", "suggestion", "layout"]);
* @property {number} warningCount Number of warnings for the result.
* @property {number} fixableErrorCount Number of fixable errors for the result.
* @property {number} fixableWarningCount Number of fixable warnings for the result.
* @property {string=} [source] The source code of the file that was linted.
* @property {string=} [output] The source code of the file that was linted, with as many fixes applied as possible.
* @property {string} [source] The source code of the file that was linted.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsdoc/check-syntax enforces this syntax for optional parameters.

* @property {string} [output] The source code of the file that was linted, with as many fixes applied as possible.
*/

/**
Expand Down Expand Up @@ -329,7 +329,6 @@ function getRule(ruleId, configArrays) {
/**
* Collect used deprecated rules.
* @param {ConfigArray[]} usedConfigArrays The config arrays which were used.
* @param {Map<string, Object>} ruleMap The rule definitions which were used (built-ins).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter no longer exists.

* @returns {IterableIterator<DeprecatedRuleInfo>} Used deprecated rules.
*/
function *iterateRuleDeprecationWarnings(usedConfigArrays) {
Expand Down Expand Up @@ -530,7 +529,6 @@ class CLIEngine {
/**
* Creates a new instance of the core CLI engine.
* @param {CLIEngineOptions} providedOptions The options for this instance.
* @constructor
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES2015 classes no longer need to be documented as @class or @constructor - the parser can now identify this case.

*/
constructor(providedOptions) {
const options = Object.assign(
Expand Down
6 changes: 3 additions & 3 deletions lib/cli-engine/ignored-paths.js
Expand Up @@ -78,7 +78,7 @@ function mergeDefaultOptions(options) {
return Object.assign({}, DEFAULT_OPTIONS, options);
}

/* eslint-disable valid-jsdoc */
/* eslint-disable jsdoc/check-param-names, jsdoc/require-param */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A really nice thing about the separate checks is that we can disable specific checks and still enforce other things we care about!

/**
* Normalize the path separators in a given string.
* On Windows environment, this replaces `\` by `/`.
Expand All @@ -89,7 +89,7 @@ function mergeDefaultOptions(options) {
const normalizePathSeps = path.sep === "/"
? (str => str)
: ((seps, str) => str.replace(seps, "/")).bind(null, new RegExp(`\\${path.sep}`, "gu"));
/* eslint-enable valid-jsdoc */
/* eslint-enable jsdoc/check-param-names, jsdoc/require-param */

/**
* Converts a glob pattern to a new glob pattern relative to a different directory
Expand Down Expand Up @@ -298,7 +298,7 @@ class IgnoredPaths {

/**
* read ignore filepath
* @param {string} filePath, file to add to ig
* @param {string} filePath file to add to ig
* @returns {Array} raw ignore rules
*/
readIgnoreFile(filePath) {
Expand Down
1 change: 0 additions & 1 deletion lib/cli-engine/lint-result-cache.js
Expand Up @@ -47,7 +47,6 @@ class LintResultCache {

/**
* Creates a new LintResultCache instance.
* @constructor
* @param {string} cacheFileLocation The cache file location.
* configuration lookup by file path).
*/
Expand Down
4 changes: 2 additions & 2 deletions lib/init/config-rule.js
Expand Up @@ -99,7 +99,7 @@ function groupByProperty(objects) {
* Configs may also have one or more additional elements to specify rule
* configuration or options.
*
* @typedef {array|number} ruleConfig
* @typedef {Array|number} ruleConfig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why valid-jsdoc didn't catch this.

* @param {number} 0 The rule's severity (0, 1, 2).
*/

Expand Down Expand Up @@ -185,7 +185,7 @@ class RuleConfigSet {

/**
* Stored valid rule configurations for this instance
* @type {array}
* @type {Array}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why valid-jsdoc didn't catch this.

*/
this.ruleConfigs = configs || [];
}
Expand Down
2 changes: 1 addition & 1 deletion lib/init/npm-utils.js
Expand Up @@ -163,7 +163,7 @@ function checkDevDeps(packages) {
/**
* Check whether package.json is found in current path.
*
* @param {string=} startDir Starting directory
* @param {string} [startDir] Starting directory
* @returns {boolean} Whether a package.json is found in current path.
*/
function checkPackageJson(startDir) {
Expand Down
2 changes: 1 addition & 1 deletion lib/linter/code-path-analysis/debug-helpers.js
Expand Up @@ -21,7 +21,7 @@ const debug = require("debug")("eslint:code-path");
* @returns {string} Id of the segment.
*/
/* istanbul ignore next */
function getId(segment) { // eslint-disable-line require-jsdoc
function getId(segment) { // eslint-disable-line jsdoc/require-jsdoc
return segment.id + (segment.reachable ? "" : "!");
}

Expand Down
1 change: 0 additions & 1 deletion lib/linter/linter.js
Expand Up @@ -472,7 +472,6 @@ function normalizeFilename(filename) {
return index === -1 ? filename : parts.slice(index).join(path.sep);
}

// eslint-disable-next-line valid-jsdoc
/**
* Normalizes the possible options for `linter.verify` and `linter.verifyAndFix` to a
* consistent shape.
Expand Down
1 change: 0 additions & 1 deletion lib/rule-tester/rule-tester.js
Expand Up @@ -177,7 +177,6 @@ class RuleTester {
/**
* Creates a new instance of RuleTester.
* @param {Object} [testerConfig] Optional, extra configuration for the tester
* @constructor
*/
constructor(testerConfig) {

Expand Down
2 changes: 1 addition & 1 deletion lib/rules/indent-legacy.js
Expand Up @@ -303,7 +303,7 @@ module.exports = {
* @param {int} needed Expected indentation character count
* @param {int} gottenSpaces Indentation space count in the actual node/code
* @param {int} gottenTabs Indentation tab count in the actual node/code
* @param {Object=} loc Error line and column location
* @param {Object} [loc] Error line and column location
* @param {boolean} isLastNodeCheck Is the error for last node check
* @returns {void}
*/
Expand Down
2 changes: 2 additions & 0 deletions lib/shared/logging.js
Expand Up @@ -12,6 +12,7 @@ module.exports = {

/**
* Cover for console.log
* @param {...any} args The elements to log.
* @returns {void}
*/
info(...args) {
Expand All @@ -20,6 +21,7 @@ module.exports = {

/**
* Cover for console.error
* @param {...any} args The elements to log.
* @returns {void}
*/
error(...args) {
Expand Down
7 changes: 3 additions & 4 deletions lib/source-code/source-code.js
Expand Up @@ -93,7 +93,6 @@ class SourceCode extends TokenStore {
* @param {ScopeManager|null} textOrConfig.scopeManager - The scope of this source code.
* @param {Object|null} textOrConfig.visitorKeys - The visitor keys to traverse AST.
* @param {ASTNode} [astIfNoConfig] - The Program node of the AST representing the code. This AST should be created from the text that BOM was stripped.
* @constructor
*/
constructor(textOrConfig, astIfNoConfig) {
let text, ast, parserServices, scopeManager, visitorKeys;
Expand Down Expand Up @@ -206,9 +205,9 @@ class SourceCode extends TokenStore {

/**
* Gets the source code for the given node.
* @param {ASTNode=} node The AST node to get the text for.
* @param {int=} beforeCount The number of characters before the node to retrieve.
* @param {int=} afterCount The number of characters after the node to retrieve.
* @param {ASTNode} [node] The AST node to get the text for.
* @param {int} [beforeCount] The number of characters before the node to retrieve.
* @param {int} [afterCount] The number of characters after the node to retrieve.
* @returns {string} The text representing the AST node.
* @public
*/
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -96,6 +96,7 @@
"eslint-config-eslint": "file:packages/eslint-config-eslint",
"eslint-plugin-eslint-plugin": "^2.0.1",
"eslint-plugin-internal-rules": "file:tools/internal-rules",
"eslint-plugin-jsdoc": "^15.9.5",
"eslint-plugin-node": "^9.0.0",
"eslint-release": "^1.2.0",
"eslump": "^2.0.0",
Expand Down
36 changes: 24 additions & 12 deletions packages/eslint-config-eslint/default.yml
@@ -1,6 +1,16 @@
extends:
- "eslint:recommended"
- "plugin:node/recommended"
plugins:
- "jsdoc"
settings:
jsdoc:
tagNamePreference:
file: "fileoverview"
augments: "extends"
class: "constructor"
preferredTypes:
object: "Object"
rules:
array-bracket-spacing: "error"
array-callback-return: "error"
Expand Down Expand Up @@ -31,6 +41,20 @@ rules:
generator-star-spacing: "error"
guard-for-in: "error"
handle-callback-err: ["error", "err"]
jsdoc/check-alignment: "error"
jsdoc/check-param-names: "error"
jsdoc/check-syntax: "error"
jsdoc/check-tag-names: "error"
jsdoc/check-types: "error"
jsdoc/implements-on-classes: "error"
jsdoc/require-jsdoc: "error"
jsdoc/require-param: "error"
jsdoc/require-param-description: "error"
jsdoc/require-param-name: "error"
jsdoc/require-param-type: "error"
jsdoc/require-returns: ["error", { forceRequireReturn: true, forceReturnsWithAsync: true }]
jsdoc/require-returns-description: "error"
jsdoc/require-returns-type: "error"
key-spacing: ["error", { beforeColon: false, afterColon: true }]
keyword-spacing: "error"
lines-around-comment: ["error", {
Expand Down Expand Up @@ -154,7 +178,6 @@ rules:
quotes: ["error", "double", {avoidEscape: true}]
quote-props: ["error", "as-needed"]
radix: "error"
require-jsdoc: "error"
require-unicode-regexp: "error"
rest-spread-spacing: "error"
semi: "error"
Expand All @@ -172,17 +195,6 @@ rules:
template-curly-spacing: ["error", "never"]
template-tag-spacing: "error"
unicode-bom: "error"
valid-jsdoc: ["error", {
prefer: { "return": "returns"},
preferType: {
"String": "string",
"Number": "number",
"Boolean": "boolean",
"array": "Array",
"object": "Object",
"function": "Function"
}
}]
wrap-iife: "error"
yield-star-spacing: "error"
yoda: ["error", "never"]
1 change: 1 addition & 0 deletions packages/eslint-config-eslint/package.json
Expand Up @@ -20,6 +20,7 @@
"homepage": "https://eslint.org",
"bugs": "https://github.com/eslint/eslint/issues/",
"peerDependencies": {
"eslint-plugin-jsdoc": "^15.9.5",
"eslint-plugin-node": "^9.0.0"
},
"keywords": [
Expand Down
2 changes: 0 additions & 2 deletions tests/lib/_utils.js
Expand Up @@ -25,7 +25,6 @@ function unIndent(strings, ...values) {
return lines.map(line => line.slice(minLineIndent)).join("\n");
}

// eslint-disable-next-line valid-jsdoc
/**
* Add support of `recursive` option.
* @param {import("fs")} fs The in-memory file system.
Expand Down Expand Up @@ -56,7 +55,6 @@ function supportMkdirRecursiveOption(fs, cwd) {
};
}

// eslint-disable-next-line valid-jsdoc
/**
* Define in-memory file system.
* @param {Object} options The options.
Expand Down
3 changes: 0 additions & 3 deletions tests/lib/cli-engine/_utils.js
Expand Up @@ -53,9 +53,6 @@
*/
"use strict";

// To use TypeScript type annotations for VSCode intellisense.
/* eslint-disable valid-jsdoc */
mysticatea marked this conversation as resolved.
Show resolved Hide resolved

const path = require("path");
const vm = require("vm");
const Proxyquire = require("proxyquire/lib/proxyquire");
Expand Down
3 changes: 3 additions & 0 deletions tests/lib/cli-engine/cascading-config-array-factory.js
Expand Up @@ -85,6 +85,7 @@ describe("CascadingConfigArrayFactory", () => {

/**
* Returns the path inside of the fixture directory.
* @param {...string} args file path segments.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem like valid-jsdoc enforced documenting rest parameters.

* @returns {string} The path inside the fixture directory.
* @private
*/
Expand Down Expand Up @@ -671,6 +672,7 @@ describe("CascadingConfigArrayFactory", () => {

/**
* Returns the path inside of the fixture directory.
* @param {...string} args file path segments.
* @returns {string} The path inside the fixture directory.
* @private
*/
Expand Down Expand Up @@ -766,6 +768,7 @@ describe("CascadingConfigArrayFactory", () => {

/**
* Returns the path inside of the fixture directory.
* @param {...string} args file path segments.
* @returns {string} The path inside the fixture directory.
* @private
*/
Expand Down
7 changes: 6 additions & 1 deletion tests/lib/cli-engine/cli-engine.js
Expand Up @@ -50,6 +50,7 @@ describe("CLIEngine", () => {

/**
* Returns the path inside of the fixture directory.
* @param {...string} args file path segments.
* @returns {string} The path inside the fixture directory.
* @private
*/
Expand Down Expand Up @@ -1543,7 +1544,11 @@ describe("CLIEngine", () => {
engine = new CLIEngine({
cwd: originalDir,
configFile: ".eslintrc.js",
rules: { "indent-legacy": 1 }
rules: {
"indent-legacy": 1,
"require-jsdoc": 1,
"valid-jsdoc": 1
}
});

const report = engine.executeOnFiles(["lib/cli*.js"]);
Expand Down
1 change: 1 addition & 0 deletions tests/lib/cli-engine/file-enumerator.js
Expand Up @@ -173,6 +173,7 @@ describe("FileEnumerator", () => {

/**
* Returns the path inside of the fixture directory.
* @param {...string} args file path segments.
* @returns {string} The path inside the fixture directory.
* @private
*/
Expand Down
1 change: 1 addition & 0 deletions tests/lib/cli-engine/ignored-paths.js
Expand Up @@ -92,6 +92,7 @@ function countDefaultPatterns(ignoredPaths) {

/**
* Returns the path inside of the fixture directory.
* @param {...string} args file path segments.
* @returns {string} The path inside the fixture directory.
* @private
*/
Expand Down
1 change: 1 addition & 0 deletions tests/lib/cli.js
Expand Up @@ -71,6 +71,7 @@ describe("cli", () => {

/**
* Returns the path inside of the fixture directory.
* @param {...string} args file path segments.
* @returns {string} The path inside the fixture directory.
* @private
*/
Expand Down
1 change: 1 addition & 0 deletions tests/lib/init/config-initializer.js
Expand Up @@ -57,6 +57,7 @@ describe("configInitializer", () => {

/**
* Returns the path inside of the fixture directory.
* @param {...string} args file path segments.
* @returns {string} The path inside the fixture directory.
* @private
*/
Expand Down
1 change: 1 addition & 0 deletions tests/lib/init/source-code-utils.js
Expand Up @@ -31,6 +31,7 @@ describe("SourceCodeUtil", () => {

/**
* Returns the path inside of the fixture directory.
* @param {...string} args file path segments.
* @returns {string} The path inside the fixture directory.
* @private
*/
Expand Down
1 change: 0 additions & 1 deletion tests/tools/loose-parser.js
Expand Up @@ -9,7 +9,6 @@
const acorn = require("acorn");
const espree = require("espree/lib/espree");

// eslint-disable-next-line valid-jsdoc
/**
* Define the parser which ignores recoverable errors.
* @returns {(parser:acorn.Parser) => acorn.Parser} The function that defines loose parser.
Expand Down
1 change: 1 addition & 0 deletions tools/code-sample-minimizer.js
Expand Up @@ -30,6 +30,7 @@ function isStatement(node) {
* Given "bad" source text (e.g. an code sample that causes a rule to crash), tries to return a smaller
* piece of source text which is also "bad", to make it easier for a human to figure out where the
* problem is.
* @param {Object} options Options to process
* @param {string} options.sourceText Initial piece of "bad" source text
* @param {function(string): boolean} options.predicate A predicate that returns `true` for bad source text and `false` for good source text
* @param {Parser} [options.parser] The parser used to parse the source text. Defaults to a modified
Expand Down
1 change: 0 additions & 1 deletion tools/internal-rules/no-invalid-meta.js
Expand Up @@ -104,7 +104,6 @@ function hasMetaSchema(metaPropertyNode) {
*
* @param {RuleContext} context The ESLint rule context.
* @param {ASTNode} exportsNode ObjectExpression node that the rule exports.
* @param {boolean} ruleIsFixable whether the rule is fixable or not.
* @returns {void}
*/
function checkMetaValidity(context, exportsNode) {
Expand Down