Skip to content

Commit

Permalink
Add expiring-todo-comments rule (sindresorhus#238)
Browse files Browse the repository at this point in the history
Fixes sindresorhus#238

* Due date check.
* Own package version check.
* Dependency package version check.
* Dependency inclusion checks.
  • Loading branch information
lubien committed May 30, 2019
1 parent 91efcba commit 9ace7fd
Show file tree
Hide file tree
Showing 6 changed files with 436 additions and 1 deletion.
56 changes: 56 additions & 0 deletions docs/rules/expiring-todo-comments.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Add conditions to TODO comments to make them error

Makes possible to pass arguments to TODO and FIXME comments to trigger errors.

Currently supporting:

* `[YYYY-MM-DD]` to define a due date.
* `[> 1]` or `[>= 2]` to expire at some version (from package.json).
* `[+package]` or `[-package]` to expire when you add/remove a package.
* `[package > 1]` or `[package >= 2]` to expire when a package hits some version.
* Stack by separating by comma such as `[YYYY-MM-DD, +package]`.
* You can either use `TODO` or `FIXME`.
* Optional author name such as `TODO (lubien) [2019-05-30]` or `TODO [2019-05-30] (lubien)`.

## Fail

```js
// TODO [2000-01-01]: forgot to refactor
// TODO [2200-12-12, 2200-12-12]: multiple dates won't work


// TODO [> 1]: if your package.json version is > 1
// TODO [>= 1]: if your package.json version is >= 1
// TODO [> 1, >2]: multiple package versions won't work

// TODO [+react]: when you install `react`, refactor to use it
// TODO [-popura]: when you uninstall `popura` do some stuff

// TODO [read-pkg > 1]: when `read-pkg` version is > 1 don't forget to do this
// TODO [read-pkg >= 5.1.1]: when `read-pkg` version is >= 5.1.1 don't forget to do that
```


## Pass

```js
// TODO [2200-12-25]: Too long... Can you feel it?'
// FIXME [2200-12-25]: Too long... Can you feel it?

// TODO (lubien) [2200-12-12]: Too long... Can you feel it?
// FIXME [2200-12-25] (lubien): Too long... Can you feel it?

// TODO [-read-pkg]: We actually use this. If we remove this package I'll error.
// TODO [+popura]: I think we wont need a broken package.

// TODO [semver > 1000]: Welp hopefully we wont get at that.
// TODO [semver >= 1000]: Welp hopefully we wont get at that.

// TODO [2200-12-25, +popura, semver > 1000]: Combo.

/*
* TODO [2200-12-25]: Yet
* TODO [2200-12-25]: Another
* TODO [2200-12-25]: Way
*/
```
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module.exports = {
'unicorn/custom-error-definition': 'off',
'unicorn/error-message': 'error',
'unicorn/escape-case': 'error',
'unicorn/expiring-todo-comments': 'error',
'unicorn/explicit-length-check': 'error',
'unicorn/filename-case': 'error',
'unicorn/import-index': 'error',
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@
"lodash.snakecase": "^4.0.1",
"lodash.topairs": "^4.3.0",
"lodash.upperfirst": "^4.2.0",
"read-pkg": "^5.1.1",
"reserved-words": "^0.1.2",
"safe-regex": "^2.0.1"
"safe-regex": "^2.0.1",
"semver": "^6.1.1"
},
"devDependencies": {
"ava": "^1.4.1",
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Configure it in `package.json`.
"unicorn/custom-error-definition": "off",
"unicorn/error-message": "error",
"unicorn/escape-case": "error",
"unicorn/expiring-todo-comments": "error",
"unicorn/explicit-length-check": "error",
"unicorn/filename-case": "error",
"unicorn/import-index": "error",
Expand Down Expand Up @@ -82,6 +83,7 @@ Configure it in `package.json`.
- [custom-error-definition](docs/rules/custom-error-definition.md) - Enforce correct `Error` subclassing. *(fixable)*
- [error-message](docs/rules/error-message.md) - Enforce passing a `message` value when throwing a built-in error.
- [escape-case](docs/rules/escape-case.md) - Require escape sequences to use uppercase values. *(fixable)*
- [expiring-todo-comments](docs/rules/expiring-todo-comments.md) - Add conditions to TODO comments to make them error.
- [explicit-length-check](docs/rules/explicit-length-check.md) - Enforce explicitly comparing the `length` property of a value. *(partly fixable)*
- [filename-case](docs/rules/filename-case.md) - Enforce a case style for filenames.
- [import-index](docs/rules/import-index.md) - Enforce importing index files with `.`. *(fixable)*
Expand Down
260 changes: 260 additions & 0 deletions rules/expiring-todo-comments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
'use strict';
const readPkg = require('read-pkg');
const semver = require('semver');
const getDocsUrl = require('./utils/get-docs-url');

const pkg = readPkg.sync();

const pkgDependencies = {...pkg.dependencies, ...pkg.devDependencies};

const TODO_RE = /[TODO|FIXME]\s*\[([^}]+)\]/i;
const DEPENDENCY_INCLUSION_RE = /^[+|-]\s*@?[\S+]\/?\S+/;
const DEPENDENCY_VERSION_RE = /(@?[\S+]\/?\S+)\s+(>|>=)\s+([\d]+(\.\d+){0,2})\s*$/;
const PKG_VERSION_RE = /(>|>=)\s*([\d]+(\.\d+){0,2})\s*$/;
const ISO8601 = /(\d{4})-(\d{2})-(\d{2})/;

const create = context => {
const sourceCode = context.getSourceCode();

function processComment(comment) {
const parsed = parseTodoWithArgs(comment.value);

if (!parsed) {
return false;
}

const {
packageVersions = [],
dates = [],
dependencies = []
} = parsed;

if (dates.length > 1) {
context.report({
node: null,
loc: comment.loc,
messageId: 'avoidMultipleDates',
data: {
expirationDates: dates.join(', ')
}
});
} else if (dates.length === 1) {
const [date] = dates;

if (reachedDate(date)) {
context.report({
node: null,
loc: comment.loc,
messageId: 'expiredTodo',
data: {
expirationDate: date
}
});
}
}

if (packageVersions.length > 1) {
context.report({
node: null,
loc: comment.loc,
messageId: 'avoidMultiplePackageVersions',
data: {
versions: packageVersions.map(({condition, version}) => `${condition} ${version}`).join(', ')
}
});
} else if (packageVersions.length === 1) {
const [{condition, version}] = packageVersions;

const pkgVersion = tryToCoerceVersion(pkg.version);
const desidedPkgVersion = tryToCoerceVersion(version);

const compare = semverComparisonForOperator(condition);

if (compare(pkgVersion, desidedPkgVersion)) {
context.report({
node: null,
loc: comment.loc,
messageId: 'reachedPackageVersion',
data: {
comparison: `${condition} ${version}`
}
});
}
}

// Inclusion: 'in', 'out'
// Comparison: '>', '>='
for (const dependency of dependencies) {
const targetPackageRawVersion = pkgDependencies[dependency.name];
const hasTargetPackage = Boolean(targetPackageRawVersion);

const isInclusion = ['in', 'out'].includes(dependency.condition);
if (isInclusion) {
const [trigger, messageId] = dependency.condition === 'in' ?
[hasTargetPackage, 'havePackage'] :
[!hasTargetPackage, 'dontHavePackage'];

if (trigger) {
context.report({
node: null,
loc: comment.loc,
messageId,
data: {
package: dependency.name
}
});
}

continue;
}

const todoVersion = tryToCoerceVersion(dependency.version);
const targetPackageVersion = tryToCoerceVersion(targetPackageRawVersion);

if (!hasTargetPackage || !targetPackageVersion) {
// Can't compare ¯\_(ツ)_/¯
continue;
}

const compare = semverComparisonForOperator(dependency.condition);

if (compare(targetPackageVersion, todoVersion)) {
context.report({
node: null,
loc: comment.loc,
messageId: 'versionMatches',
data: {
comparison: `${dependency.name} ${dependency.condition} ${dependency.version}`
}
});
}
}
}

return {
Program() {
const comments = sourceCode.getAllComments();
comments.filter(token => token.type !== 'Shebang').forEach(processComment);
}
};
};

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
url: getDocsUrl(__filename)
},
messages: {
avoidMultipleDates: 'Avoid using multiple expiration dates for TODO {{ expirationDates }}',
expiredTodo: 'You have a TODO that past due date {{ expirationDate }}',
reachedPackageVersion: 'You have a TODO that past due package version {{ comparison }}',
avoidMultiplePackageVersions: 'Avoid asking multiple package versions for TODO {{ versions }}',
havePackage: 'You have a TODO that is deprecated since you installed {{ package }}',
dontHavePackage: 'You have a TODO that is deprecated since you uninstalled {{ package }}',
versionMatches: 'You have a TODO matches version for package {{ comparison }}'
}
}
};

function parseTodoWithArgs(str) {
const result = TODO_RE.exec(str);

if (!result) {
return false;
}

const rawArgs = result[1];

return rawArgs
.split(',')
.map(arg => parseArg(arg.trim()))
.reduce((groups, arg) => {
if (!groups[arg.type]) {
groups[arg.type] = [];
}

groups[arg.type].push(arg.value);
return groups;
}, {});
}

function parseArg(argString) {
if (ISO8601.test(argString)) {
return {
type: 'dates',
value: argString
};
}

if (DEPENDENCY_INCLUSION_RE.test(argString)) {
const condition = argString[0] === '+' ? 'in' : 'out';
const name = argString.substring(1).trim();

return {
type: 'dependencies',
value: {
name,
condition
}
};
}

if (DEPENDENCY_VERSION_RE.test(argString)) {
const result = DEPENDENCY_VERSION_RE.exec(argString);
const name = result[1].trim();
const condition = result[2].trim();
const version = result[3].trim();

return {
type: 'dependencies',
value: {
name,
condition,
version
}
};
}

if (PKG_VERSION_RE.test(argString)) {
const result = PKG_VERSION_RE.exec(argString);
const condition = result[1].trim();
const version = result[2].trim();

return {
type: 'packageVersions',
value: {
condition,
version
}
};
}

// Currently being ignored as integration tests pointed
// some TODO comments have [random data like this]
return {
type: 'unknowns',
value: argString
};
}

function reachedDate(past) {
const now = (new Date()).toISOString().substring(0, 10);
return Date.parse(past) < Date.parse(now);
}

function tryToCoerceVersion(version) {
try {
return semver.coerce(version);
} catch (error) {
return false;
}
}

function semverComparisonForOperator(operator) {
return ({
'>': semver.gt,
'>=': semver.gte
})[operator];
}
Loading

0 comments on commit 9ace7fd

Please sign in to comment.