Skip to content

Commit

Permalink
feat(require-returns): move settings to options
Browse files Browse the repository at this point in the history
BREAKING CHANGE: The following settings has been removed and became options
    under relevant rules.
    * `settings.jsdoc.allowEmptyNamepaths` -> option in `valid-types`
    * `settings.jsdoc.checkSeesForNamepaths` -> option in `valid-types`
    * `settings.jsdoc.exemptEmptyFunctions` -> option in `require-jsdoc`
    * `settings.jsdoc.forceRequireReturn` -> option in `require-returns`
    * `settings.jsdoc.avoidExampleOnConstructors` -> option in `require-example`
  • Loading branch information
golopot committed Jul 7, 2019
1 parent ed56935 commit 76e1e97
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 62 deletions.
3 changes: 0 additions & 3 deletions src/iterateJsdoc.js
Expand Up @@ -281,9 +281,6 @@ const getSettings = (context) => {
settings.allowEmptyNamepaths = _.get(context, 'settings.jsdoc.allowEmptyNamepaths') !== false;
settings.checkSeesForNamepaths = Boolean(_.get(context, 'settings.jsdoc.checkSeesForNamepaths'));

// `require-returns` only
settings.forceRequireReturn = Boolean(_.get(context, 'settings.jsdoc.forceRequireReturn'));

// `require-example` only
settings.avoidExampleOnConstructors = Boolean(_.get(context, 'settings.jsdoc.avoidExampleOnConstructors'));

Expand Down
17 changes: 13 additions & 4 deletions src/rules/requireReturns.js
@@ -1,4 +1,5 @@
import iterateJsdoc from '../iterateJsdoc';
import warnRemovedSettings from '../warnRemovedSettings';

/**
* We can skip checking for a return value, in case the documentation is inherited
Expand Down Expand Up @@ -43,16 +44,20 @@ const canSkip = (utils) => {
export default iterateJsdoc(({
report,
utils,
context,
settings
context
}) => {
warnRemovedSettings(context, 'require-returns');

// A preflight check. We do not need to run a deep check
// in case the @returns comment is optional or undefined.
if (canSkip(utils)) {
return;
}

const options = context.options[0] || {};
const options = context.options[0] || {
forceRequireReturn: false,
forceReturnsWithAsync: false
};

const tagName = utils.getPreferredTagName('returns');
if (!tagName) {
Expand All @@ -68,7 +73,7 @@ export default iterateJsdoc(({
const [tag] = tags;
const missingReturnTag = typeof tag === 'undefined' || tag === null;
if (missingReturnTag &&
((utils.isAsync() && !utils.hasReturnValue(true) ? options.forceReturnsWithAsync : utils.hasReturnValue()) || settings.forceRequireReturn)
((utils.isAsync() && !utils.hasReturnValue(true) ? options.forceReturnsWithAsync : utils.hasReturnValue()) || options.forceRequireReturn)
) {
report(`Missing JSDoc @${tagName} declaration.`);
}
Expand All @@ -84,6 +89,10 @@ export default iterateJsdoc(({
},
type: 'array'
},
forceRequireReturn: {
default: false,
type: 'boolean'
},
forceReturnsWithAsync: {
default: false,
type: 'boolean'
Expand Down
107 changes: 107 additions & 0 deletions src/warnRemovedSettings.js
@@ -0,0 +1,107 @@
/**
* @typedef {(
* | "require-jsdoc"
* | "require-returns"
* | "valid-types"
* | "require-example"
* | "check-examples"
* )} RulesWithMovedSettings
*/

/** @type {WeakMap<Object, Set<string>>} */
const warnedSettings = new WeakMap();

/**
* Warn only once for each context and setting
*
* @param {Object} context
* @param {string} setting
*/
const hasBeenWarned = (context, setting) => {
return warnedSettings.has(context) && warnedSettings.get(context).has(setting);
};

const markSettingAsWarned = (context, setting) => {
if (!warnedSettings.has(context)) {
warnedSettings.set(context, new Set());
}

warnedSettings.get(context).add(setting);
};

/**
* @param {Object} obj
* @param {string} property
* @returns {boolean}
*/
const has = (obj, property) => {
return Object.prototype.hasOwnProperty.call(obj, property);
};

/**
*
* @param {RulesWithMovedSettings} ruleName
* @returns {string[]}
*/
const getMovedSettings = (ruleName) => {
switch (ruleName) {
case 'require-jsdoc':
return ['exemptEmptyFunctions'];
case 'require-returns':
return ['forceRequireReturn'];
case 'valid-types':
return ['allowEmptyNamepaths', 'checkSeesForNamepaths'];
case 'require-example':
return ['avoidExampleOnConstructors'];

// TODO: move settings of check-examples to options
/* istanbul ignore next */
case 'check-examples':
return [
'captionRequired',
'exampleCodeRegex',
'rejectExampleCodeRegex',
'allowInlineConfig',
'noDefaultExampleRules',
'matchingFileName',
'configFile',
'eslintrcForExamples',
'baseConfig',
'reportUnusedDisableDirectives'
];
}

/* istanbul ignore next */
return [];
};

/**
* @param {Object} context
* @param {RulesWithMovedSettings} ruleName
*/
export default function warnRemovedSettings (context, ruleName) {
const movedSettings = getMovedSettings(ruleName);

if (!context.settings || !context.settings.jsdoc) {
return;
}

for (const setting of movedSettings) {
if (
has(context.settings.jsdoc, setting) &&
!hasBeenWarned(context, setting)
) {
context.report({
loc: {
start: {
column: 1,
line: 1
}
},
message: `\`settings.jsdoc.${setting}\` has been removed, ` +
`use options in the rule \`${ruleName}\` instead.`
});
markSettingAsWarned(context, setting);
}
}
}
111 changes: 56 additions & 55 deletions test/rules/assertions/requireReturns.js
Expand Up @@ -85,6 +85,29 @@ export default {
}
}
},
{
code: `
/**
*
*/
function foo() {}
/**
*
*/
function bar() {}
`,
errors: [
{
message: '`settings.jsdoc.forceRequireReturn` has been removed, use options in the rule `require-returns` instead.'
}
],
settings: {
jsdoc: {
forceRequireReturn: true
}
}
},
{
code: `
/**
Expand All @@ -99,13 +122,11 @@ export default {
message: 'Missing JSDoc @returns declaration.'
}
],
options: [{
forceRequireReturn: true
}],
parserOptions: {
ecmaVersion: 8
},
settings: {
jsdoc: {
forceRequireReturn: true
}
}
},
{
Expand All @@ -121,13 +142,11 @@ export default {
message: 'Missing JSDoc @returns declaration.'
}
],
options: [{
forceRequireReturn: true
}],
parserOptions: {
ecmaVersion: 8
},
settings: {
jsdoc: {
forceRequireReturn: true
}
}
},
{
Expand All @@ -143,13 +162,11 @@ export default {
message: 'Missing JSDoc @returns declaration.'
}
],
options: [{
forceRequireReturn: true
}],
parserOptions: {
ecmaVersion: 8
},
settings: {
jsdoc: {
forceRequireReturn: true
}
}
},
{
Expand All @@ -165,13 +182,11 @@ export default {
message: 'Missing JSDoc @returns declaration.'
}
],
options: [{
forceRequireReturn: true
}],
parserOptions: {
ecmaVersion: 8
},
settings: {
jsdoc: {
forceRequireReturn: true
}
}
},
{
Expand All @@ -188,11 +203,9 @@ export default {
message: 'Missing JSDoc @returns declaration.'
}
],
settings: {
jsdoc: {
forceRequireReturn: true
}
}
options: [{
forceRequireReturn: true
}]
},
{
code: `
Expand Down Expand Up @@ -490,11 +503,9 @@ export default {
}
}
`,
settings: {
jsdoc: {
forceRequireReturn: true
}
}
options: [{
forceRequireReturn: true
}]
},
{
code: `
Expand All @@ -516,11 +527,9 @@ export default {
function quux () {
}
`,
settings: {
jsdoc: {
forceRequireReturn: true
}
}
options: [{
forceRequireReturn: true
}]
},
{
code: `
Expand All @@ -541,11 +550,9 @@ export default {
return undefined;
}
`,
settings: {
jsdoc: {
forceRequireReturn: true
}
}
options: [{
forceRequireReturn: true
}]
},
{
code: `
Expand All @@ -565,11 +572,9 @@ export default {
function quux () {
}
`,
settings: {
jsdoc: {
forceRequireReturn: true
}
}
options: [{
forceRequireReturn: true
}]
},
{
code: `
Expand All @@ -580,11 +585,9 @@ export default {
return;
}
`,
settings: {
jsdoc: {
forceRequireReturn: true
}
}
options: [{
forceRequireReturn: true
}]
},
{
code: `
Expand All @@ -602,13 +605,11 @@ export default {
async function quux () {
}
`,
options: [{
forceRequireReturn: true
}],
parserOptions: {
ecmaVersion: 8
},
settings: {
jsdoc: {
forceRequireReturn: true
}
}
},
{
Expand Down

0 comments on commit 76e1e97

Please sign in to comment.