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

Build: Fix JSDoc syntax errors #9813

Merged
merged 2 commits into from Jan 20, 2018
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
28 changes: 11 additions & 17 deletions lib/linter.js
Expand Up @@ -48,6 +48,14 @@ const MAX_AUTOFIX_PASSES = 10;
* @property {Object|null} visitorKeys The visitor keys to traverse this AST.
*/

/**
* @typedef {Object} DisableDirective
* @property {("disable"|"enable"|"disable-line"|"disable-next-line")} type
* @property {number} line
* @property {number} column
* @property {(string|null)} ruleId
*/

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -260,12 +268,7 @@ function addDeclaredGlobals(globalScope, config, envContext) {
* @param {{line: number, column: number}} loc The 0-based location of the comment token
* @param {string} value The value after the directive in the comment
* comment specified no specific rules, so it applies to all rules (e.g. `eslint-disable`)
* @returns {{
* type: ("disable"|"enable"|"disable-line"|"disable-next-line"),
* line: number,
* column: number,
* ruleId: (string|null)
* }[]} Directives from the comment
* @returns {DisableDirective[]} Directives from the comment
*/
function createDisableDirectives(type, loc, value) {
const ruleIds = Object.keys(parseListConfig(value));
Expand All @@ -282,17 +285,8 @@ function createDisableDirectives(type, loc, value) {
* @param {ASTNode} ast The top node of the AST.
* @param {Object} config The existing configuration data.
* @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
* @returns {{
* config: Object,
* problems: Problem[],
* disableDirectives: {
* type: ("disable"|"enable"|"disable-line"|"disable-next-line"),
* line: number,
* column: number,
* ruleId: (string|null)
* }[]
* }} Modified config object, along with any problems encountered
* while parsing config comments
* @returns {{config: Object, problems: Problem[], disableDirectives: DisableDirective[]}}
* Modified config object, along with any problems encountered while parsing config comments
*/
function modifyConfigsFromComments(filename, ast, config, ruleMapper) {

Expand Down
65 changes: 28 additions & 37 deletions lib/report-translator.js
Expand Up @@ -28,6 +28,22 @@ const interpolate = require("./util/interpolate");
* @property {Function} [fix] The function to call that creates a fix command.
*/

/**
* Information about the report
* @typedef {Object} ReportInfo
* @property {string} ruleId
* @property {(0|1|2)} severity
* @property {(string|undefined)} message
* @property {(string|undefined)} messageId
* @property {number} line
* @property {number} column
* @property {(number|undefined)} endLine
* @property {(number|undefined)} endColumn
* @property {(string|null)} nodeType
* @property {string} source
* @property {({text: string, range: (number[]|null)}|null)} fix
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if this should be extracted to a typedef? It's a bit gnarly.

Copy link
Contributor

Choose a reason for hiding this comment

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

@property {Fix|null} fix maybe?

*/

//------------------------------------------------------------------------------
// Module Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -121,7 +137,7 @@ function compareFixesByRange(a, b) {
* Merges the given fixes array into one.
* @param {Fix[]} fixes The fixes to merge.
* @param {SourceCode} sourceCode The source code object to get the text between fixes.
* @returns {{text: string, range: [number, number]}} The merged fixes
* @returns {{text: string, range: number[]}} The merged fixes
Copy link
Member

Choose a reason for hiding this comment

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

Not familiar with JSDoc. Can we do something like number[2] or otherwise represent the ranges as a 2-tuple somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately it doesn't work, and I haven't found any details about specifying the length of the array on the internet. I don't think JSDoc supports it.

*/
function mergeFixes(fixes, sourceCode) {
if (fixes.length === 0) {
Expand Down Expand Up @@ -158,7 +174,7 @@ function mergeFixes(fixes, sourceCode) {
* If the descriptor retrieves multiple fixes, this merges those to one.
* @param {MessageDescriptor} descriptor The report descriptor.
* @param {SourceCode} sourceCode The source code object to get text between fixes.
* @returns {({text: string, range: [number, number]}|null)} The fix for the descriptor
* @returns {({text: string, range: number[]}|null)} The fix for the descriptor
*/
function normalizeFixes(descriptor, sourceCode) {
if (typeof descriptor.fix !== "function") {
Expand All @@ -177,27 +193,15 @@ function normalizeFixes(descriptor, sourceCode) {

/**
* Creates information about the report from a descriptor
* @param {{
* ruleId: string,
* severity: (0|1|2),
* node: (ASTNode|null),
* message: string,
* loc: {start: SourceLocation, end: (SourceLocation|null)},
* fix: ({text: string, range: [number, number]}|null),
* sourceLines: string[]
* }} options Information about the problem
* @returns {function(...args): {
* ruleId: string,
* severity: (0|1|2),
* message: string,
* line: number,
* column: number,
* endLine: (number|undefined),
* endColumn: (number|undefined),
* nodeType: (string|null),
* source: string,
* fix: ({text: string, range: [number, number]}|null)
* }} Information about the report
* @param {Object} options Information about the problem
* @param {string} options.ruleId Rule ID
* @param {(0|1|2)} options.severity Rule severity
* @param {(ASTNode|null)} options.node Node
* @param {string} options.message Error message
* @param {{start: SourceLocation, end: (SourceLocation|null)}} options.loc Start and end location
* @param {{text: string, range: (number[]|null)}} options.fix The fix object
* @param {string[]} options.sourceLines Source lines
* @returns {function(...args): ReportInfo} Function that returns information about the report
*/
function createProblem(options) {
const problem = {
Expand Down Expand Up @@ -235,20 +239,7 @@ function createProblem(options) {
* problem for the Node.js API.
* @param {{ruleId: string, severity: number, sourceCode: SourceCode, messageIds: Object}} metadata Metadata for the reported problem
* @param {SourceCode} sourceCode The `SourceCode` instance for the text being linted
* @returns {function(...args): {
* ruleId: string,
* severity: (0|1|2),
* message: (string|undefined),
* messageId: (string|undefined),
* line: number,
* column: number,
* endLine: (number|undefined),
* endColumn: (number|undefined),
* nodeType: (string|null),
* source: string,
* fix: ({text: string, range: [number, number]}|null)
* }}
* Information about the report
* @returns {function(...args): ReportInfo} Function that returns information about the report
*/

module.exports = function createReportTranslator(metadata) {
Expand Down