Skip to content

valid-types: disallow namepath on interface tag for Closure #587

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 1 commit 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
2 changes: 2 additions & 0 deletions .README/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 @@ -13430,6 +13432,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
40 changes: 29 additions & 11 deletions src/jsdocUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,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 +386,6 @@ const namepathDefiningTags = new Set([
'class', 'constructor',
'constant', 'const',
'function', 'func', 'method',
'interface',
'member', 'var',
'mixin',
'namespace',
Expand All @@ -400,11 +399,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 +434,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 +477,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 +501,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
34 changes: 34 additions & 0 deletions test/rules/assertions/validTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,40 @@ export default {
},
},
},
{
code: `
/**
* @interface name<
*/
`,
errors: [
{
message: 'Syntax error in namepath: name<',
},
],
settings: {
jsdoc: {
mode: 'jsdoc',
},
},
},
{
code: `
/**
* @interface name
*/
`,
errors: [
{
message: '@interface should not have a name in "closure" mode.',
},
],
settings: {
jsdoc: {
mode: 'closure',
},
},
},
],
valid: [
{
Expand Down