Skip to content

Check tag names closure #588

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

Merged
merged 2 commits into from
Jun 23, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .README/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ how many line breaks to add when a block is missing.
containing both JavaScript and TypeScript, you can also use [`overrides`](https://eslint.org/docs/user-guide/configuring). You may also set to `"permissive"` to
try to be as accommodating to any of the styles, but this is not recommended.
Currently is used for the following:
- Determine valid tags for `check-tag-names`
- Determine valid tags and aliases for `check-tag-names`
- Only check `@template` in `no-undefined-types` for types in "closure" and
"typescript" modes
- For type-checking rules, determine which tags will be checked for types
Expand All @@ -144,6 +144,8 @@ how many line breaks to add when a block is missing.
- For type-checking rules, impacts parsing of types (through
[jsdoctypeparser](https://github.com/jsdoctypeparser/jsdoctypeparser) dependency)
- Check preferred tag names
- Disallows namepath on `@interface` for "closure" mode in `valid-types` (and
avoids checking in other rules)

### Alias Preference

Expand Down
3 changes: 2 additions & 1 deletion .README/rules/check-tag-names.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ template

And for [Closure](https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler),
when `settings.jsdoc.mode` is set to `closure`, one may use the following (in
addition to the jsdoc and TypeScript tags):
addition to the jsdoc and TypeScript tags–though replacing `returns` with
`return`):

```
define (synonym of `const` per jsdoc source)
Expand Down
3 changes: 2 additions & 1 deletion .README/rules/valid-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ Also impacts behaviors on namepath (or event)-defining and pointing tags:
`@class`, `@constructor`, `@constant`, `@const`,
`@function`, `@func`, `@method`, `@interface`, `@member`, `@var`,
`@mixin`, `@namespace`
1. Name(path)-pointing tags requiring namepath: `@alias`, `@augments`, `@extends`, `@lends`, `@memberof`, `@memberof!`, `@mixes`, `@this`
1. Name(path)-pointing tags requiring namepath: `@alias`, `@augments`,
`@extends`, `@lends`, `@memberof`, `@memberof!`, `@mixes`, `@this`
1. Name(path)-pointing tags (which may have value without namepath or their
namepath can be expressed elsewhere on the block): `@listens`, `@fires`,
`@emits`, and `@modifies`
Expand Down
40 changes: 37 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ how many line breaks to add when a block is missing.
containing both JavaScript and TypeScript, you can also use [`overrides`](https://eslint.org/docs/user-guide/configuring). You may also set to `"permissive"` to
try to be as accommodating to any of the styles, but this is not recommended.
Currently is used for the following:
- Determine valid tags for `check-tag-names`
- Determine valid tags and aliases for `check-tag-names`
- Only check `@template` in `no-undefined-types` for types in "closure" and
"typescript" modes
- For type-checking rules, determine which tags will be checked for types
Expand All @@ -202,6 +202,8 @@ how many line breaks to add when a block is missing.
- For type-checking rules, impacts parsing of types (through
[jsdoctypeparser](https://github.com/jsdoctypeparser/jsdoctypeparser) dependency)
- Check preferred tag names
- Disallows namepath on `@interface` for "closure" mode in `valid-types` (and
avoids checking in other rules)

<a name="eslint-plugin-jsdoc-settings-alias-preference"></a>
### Alias Preference
Expand Down Expand Up @@ -2629,7 +2631,8 @@ template

And for [Closure](https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler),
when `settings.jsdoc.mode` is set to `closure`, one may use the following (in
addition to the jsdoc and TypeScript tags):
addition to the jsdoc and TypeScript tags–though replacing `returns` with
`return`):

```
define (synonym of `const` per jsdoc source)
Expand Down Expand Up @@ -2861,6 +2864,13 @@ function quux () {
// Settings: {"jsdoc":{"tagNamePreference":{"abc":"abcd"}}}
// Message: Invalid JSDoc tag (preference). Replace "abc" JSDoc tag with "abcd".

/**
* @returns
*/
function quux (foo) {}
// Settings: {"jsdoc":{"mode":"closure"}}
// Message: Invalid JSDoc tag (preference). Replace "returns" JSDoc tag with "return".

/**
* @modifies
* @abstract
Expand Down Expand Up @@ -3069,6 +3079,17 @@ function quux (foo) {
}
// Settings: {"jsdoc":{"tagNamePreference":{"param":"baz","returns":{"message":"Prefer `bar`","replacement":"bar"},"todo":false}}}

/**
* @returns
*/
function quux (foo) {}

/**
* @return
*/
function quux (foo) {}
// Settings: {"jsdoc":{"mode":"closure"}}

/**
* @modifies
* @abstract
Expand Down Expand Up @@ -13212,7 +13233,8 @@ Also impacts behaviors on namepath (or event)-defining and pointing tags:
`@class`, `@constructor`, `@constant`, `@const`,
`@function`, `@func`, `@method`, `@interface`, `@member`, `@var`,
`@mixin`, `@namespace`
1. Name(path)-pointing tags requiring namepath: `@alias`, `@augments`, `@extends`, `@lends`, `@memberof`, `@memberof!`, `@mixes`, `@this`
1. Name(path)-pointing tags requiring namepath: `@alias`, `@augments`,
`@extends`, `@lends`, `@memberof`, `@memberof!`, `@mixes`, `@this`
1. Name(path)-pointing tags (which may have value without namepath or their
namepath can be expressed elsewhere on the block): `@listens`, `@fires`,
`@emits`, and `@modifies`
Expand Down Expand Up @@ -13430,6 +13452,18 @@ function quux () {}
function foo(bar) {}
// Settings: {"jsdoc":{"mode":"jsdoc"}}
// Message: Syntax error in type: [number, string]

/**
* @interface name<
*/
// Settings: {"jsdoc":{"mode":"jsdoc"}}
// Message: Syntax error in namepath: name<

/**
* @interface name
*/
// Settings: {"jsdoc":{"mode":"closure"}}
// Message: @interface should not have a name in "closure" mode.
````

The following patterns are not considered problems:
Expand Down
6 changes: 3 additions & 3 deletions src/iterateJsdoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ const getUtils = (

const exemptedBy = context.options[0]?.exemptedBy ?? [
'inheritDoc',
...settings.mode === 'closure' ? [] : ['inheritdoc'],
...mode === 'closure' ? [] : ['inheritdoc'],
];
if (exemptedBy.length && utils.getPresentTags(exemptedBy).length) {
return true;
Expand All @@ -302,7 +302,7 @@ const getUtils = (
};

utils.tagMightHaveNamePosition = (tagName) => {
return jsdocUtils.tagMightHaveNamePosition(tagName);
return jsdocUtils.tagMightHaveNamePosition(mode, tagName);
};

utils.tagMustHaveTypePosition = (tagName) => {
Expand All @@ -314,7 +314,7 @@ const getUtils = (
};

utils.isNamepathDefiningTag = (tagName) => {
return jsdocUtils.isNamepathDefiningTag(tagName);
return jsdocUtils.isNamepathDefiningTag(mode, tagName);
};

utils.hasDefinedTypeReturnTag = (tag) => {
Expand Down
44 changes: 32 additions & 12 deletions src/jsdocUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,9 @@ const getPreferredTagName = (

// Allow keys to have a 'tag ' prefix to avoid upstream bug in ESLint
// that disallows keys that conflict with Object.prototype,
// e.g. 'tag constructor' for 'constructor' (#537)
// e.g. 'tag constructor' for 'constructor':
// https://github.com/eslint/eslint/issues/13289
// https://github.com/gajus/eslint-plugin-jsdoc/issues/537
const tagPreferenceFixed = _.mapKeys(tagPreference, (value, key) => {
return key.replace('tag ', '');
});
Expand Down Expand Up @@ -375,7 +377,7 @@ const tagsWithOptionalTypePositionClosure = new Set([
]);

// None of these show as having curly brackets for their name/namepath
const namepathDefiningTags = new Set([
const closureNamepathDefiningTags = new Set([
// These appear to require a "name" in their signature, albeit these
// are somewhat different from other "name"'s (including as described
// at https://jsdoc.app/about-namepaths.html )
Expand All @@ -386,7 +388,6 @@ const namepathDefiningTags = new Set([
'class', 'constructor',
'constant', 'const',
'function', 'func', 'method',
'interface',
'member', 'var',
'mixin',
'namespace',
Expand All @@ -400,11 +401,14 @@ const namepathDefiningTags = new Set([
'typedef',
'callback',
]);
const namepathDefiningTags = new Set([
...closureNamepathDefiningTags,

// The following do not seem to allow curly brackets in their doc
// signature or examples (besides `modifies` and `param`)
const tagsWithOptionalNamePosition = new Set([
...namepathDefiningTags,
// Allows for "name" in signature, but indicates as optional
'interface',
]);

const tagsWithOptionalNamePositionBase = new Set([
'param',

// `borrows` has a different format, however, so needs special parsing;
Expand Down Expand Up @@ -432,6 +436,18 @@ const tagsWithOptionalNamePosition = new Set([
'see',
]);

// The following do not seem to allow curly brackets in their doc
// signature or examples (besides `modifies` and `param`)
const tagsWithOptionalNamePosition = new Set([
...namepathDefiningTags,
...tagsWithOptionalNamePositionBase,
]);

const closureTagsWithOptionalNamePosition = new Set([
...closureNamepathDefiningTags,
...tagsWithOptionalNamePositionBase,
]);

// Todo: `@link` seems to require a namepath OR URL and might be checked as such.

// The doc signature of `event` seems to require a "name"
Expand Down Expand Up @@ -463,8 +479,10 @@ const tagsWithMandatoryTypeOrNamePosition = new Set([
'mixes',
]);

const isNamepathDefiningTag = (tagName) => {
return namepathDefiningTags.has(tagName);
const isNamepathDefiningTag = (mode, tagName) => {
return mode === 'closure' ?
closureNamepathDefiningTags.has(tagName) :
namepathDefiningTags.has(tagName);
};

const tagMightHaveTypePosition = (mode, tag) => {
Expand All @@ -485,16 +503,18 @@ const tagMustHaveTypePosition = (mode, tag) => {
return tagsWithMandatoryTypePosition.has(tag);
};

const tagMightHaveNamePosition = (tag) => {
return tagsWithOptionalNamePosition.has(tag);
const tagMightHaveNamePosition = (mode, tag) => {
return mode === 'closure' ?
closureTagsWithOptionalNamePosition.has(tag) :
tagsWithOptionalNamePosition.has(tag);
};

const tagMustHaveNamePosition = (tag) => {
return tagsWithMandatoryNamePosition.has(tag);
};

const tagMightHaveEitherTypeOrNamePosition = (mode, tag) => {
return tagMightHaveTypePosition(mode, tag) || tagMightHaveNamePosition(tag);
return tagMightHaveTypePosition(mode, tag) || tagMightHaveNamePosition(mode, tag);
};

const tagMustHaveEitherTypeOrNamePosition = (tag) => {
Expand Down
17 changes: 15 additions & 2 deletions src/rules/validTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default iterateJsdoc(({
if (!jsdoc.tags) {
return;
}
// eslint-disable-next-line complexity
jsdoc.tags.forEach((tag) => {
const validNamepathParsing = function (namepath, tagName) {
try {
Expand Down Expand Up @@ -80,7 +81,8 @@ export default iterateJsdoc(({
const hasEither = utils.tagMightHaveEitherTypeOrNamePosition(tag.tag) && (hasTypePosition || hasNameOrNamepathPosition);
const mustHaveEither = utils.tagMustHaveEitherTypeOrNamePosition(tag.tag);

if (tag.tag === 'borrows') {
switch (tag.tag) {
case 'borrows': {
const thisNamepath = tag.description.replace(asExpression, '');

if (!asExpression.test(tag.description) || !thisNamepath) {
Expand All @@ -94,7 +96,17 @@ export default iterateJsdoc(({

validNamepathParsing(thatNamepath);
}
} else {
break;
}
case 'interface': {
if (mode === 'closure' && tag.name) {
report('@interface should not have a name in "closure" mode.', null, tag);
break;
}
}

// Fallthrough
default: {
if (mustHaveEither && !hasEither && !mustHaveTypePosition) {
report(`Tag @${tag.tag} must have either a type or namepath`, null, tag);

Expand All @@ -113,6 +125,7 @@ export default iterateJsdoc(({
report(`Tag @${tag.tag} must have a name/namepath`, null, tag);
}
}
}
});
}, {
iterateAllJsdocs: true,
Expand Down
14 changes: 11 additions & 3 deletions src/tagNames.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,17 @@ const undocumentedClosureTags = {
};

const {
// eslint-disable-next-line no-unused-vars
/* eslint-disable no-unused-vars */
inheritdoc,
...typeScriptTagsNoInheritdoc

// Will be inverted to prefer `return`
returns,
/* eslint-enable no-unused-vars */
...typeScriptTagsInClosure
} = typeScriptTags;

const closureTags = {
...typeScriptTagsNoInheritdoc,
...typeScriptTagsInClosure,
...undocumentedClosureTags,

// From https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler
Expand Down Expand Up @@ -180,6 +184,10 @@ const closureTags = {
// Defined as a synonym of `interface` in jsdoc `definitions.js`
record: [],

return: [
'returns',
],

struct: [],
suppress: [],

Expand Down
45 changes: 45 additions & 0 deletions test/rules/assertions/checkTagNames.js
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,30 @@ export default {
},
},
},
{
code: `
/**
* @returns
*/
function quux (foo) {}
`,
errors: [
{
message: 'Invalid JSDoc tag (preference). Replace "returns" JSDoc tag with "return".',
},
],
output: `
/**
* @return
*/
function quux (foo) {}
`,
settings: {
jsdoc: {
mode: 'closure',
},
},
},
{
code: `${ALL_JSDOC_TAGS_COMMENT}\nfunction quux (foo) {}`,
errors: [
Expand Down Expand Up @@ -619,6 +643,27 @@ export default {
},
},
},
{
code: `
/**
* @returns
*/
function quux (foo) {}
`,
},
{
code: `
/**
* @return
*/
function quux (foo) {}
`,
settings: {
jsdoc: {
mode: 'closure',
},
},
},
{
code: `${ALL_JSDOC_TAGS_COMMENT}\nfunction quux (foo) {}`,
},
Expand Down
Loading