From 0ccbb0e630552a333c28309872a3349baa39f465 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 12 Apr 2017 11:52:37 -0700 Subject: [PATCH 1/2] build: detect todos in public comments * Creates a naive rule that checks all multi-line comments and reports failures once it detects TODOs inside of it. * This means that TODOs always need to placed inside of single-line comments. Fixes #4026 --- .../observe-content/observe-content.spec.ts | 6 ++-- src/lib/select/select.ts | 1 - src/lib/tabs/tab-body.ts | 5 ++- tools/tslint-rules/noExposedTodoRule.js | 32 +++++++++++++++++++ tslint.json | 6 +++- 5 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 tools/tslint-rules/noExposedTodoRule.js diff --git a/src/lib/core/observe-content/observe-content.spec.ts b/src/lib/core/observe-content/observe-content.spec.ts index 888a37391a0e..330a812b2e7c 100644 --- a/src/lib/core/observe-content/observe-content.spec.ts +++ b/src/lib/core/observe-content/observe-content.spec.ts @@ -2,10 +2,8 @@ import {Component} from '@angular/core'; import {async, TestBed} from '@angular/core/testing'; import {ObserveContentModule} from './observe-content'; -/** - * TODO(elad): `ProxyZone` doesn't seem to capture the events raised by - * `MutationObserver` and needs to be investigated - */ +// TODO(elad): `ProxyZone` doesn't seem to capture the events raised by +// `MutationObserver` and needs to be investigated describe('Observe content', () => { beforeEach(async(() => { diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 91b1b9a47558..25535854feca 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -648,7 +648,6 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal /** * Sets the `multiple` property on each option. The promise is necessary * in order to avoid Angular errors when modifying the property after init. - * TODO: there should be a better way of doing this. */ private _setOptionMultiple() { if (this.multiple) { diff --git a/src/lib/tabs/tab-body.ts b/src/lib/tabs/tab-body.ts index ec5bbfe90018..09cb9084e2b5 100644 --- a/src/lib/tabs/tab-body.ts +++ b/src/lib/tabs/tab-body.ts @@ -153,11 +153,10 @@ export class MdTabBody implements OnInit, AfterViewChecked, AfterContentChecked * computed style (with Angular > 2.3.0). This can alternatively be determined by checking the * transform: canBeAnimated = getComputedStyle(element) !== '', however document.contains should * be faster since it doesn't cause a reflow. - * - * TODO: This can safely be removed after we stop supporting Angular < 2.4.2. The fix landed via - * https://github.com/angular/angular/commit/21030e9a1cf30e8101399d8535ed72d847a23ba6 */ ngAfterContentChecked() { + // TODO: This can safely be removed after we stop supporting Angular < 2.4.2. The fix landed via + // https://github.com/angular/angular/commit/21030e9a1cf30e8101399d8535ed72d847a23ba6 if (!this._canBeAnimated) { this._canBeAnimated = document.body.contains(this._elementRef.nativeElement); diff --git a/tools/tslint-rules/noExposedTodoRule.js b/tools/tslint-rules/noExposedTodoRule.js new file mode 100644 index 000000000000..b67f5dbc4fd0 --- /dev/null +++ b/tools/tslint-rules/noExposedTodoRule.js @@ -0,0 +1,32 @@ +const ts = require('typescript'); +const utils = require('tsutils'); +const Lint = require('tslint'); + +const ERROR_MESSAGE = 'Todo inside of the comment might be published in the docs.'; + +/** + * Rule that walks through all comments inside of the library and adds failures when it + * detects TODO's inside of multi-line comments. TODOs need to be placed inside of single-line + * comments. + */ +class Rule extends Lint.Rules.AbstractRule { + + apply(sourceFile) { + return this.applyWithWalker(new NoExposedTodoWalker(sourceFile, this.getOptions())); + } +} + +class NoExposedTodoWalker extends Lint.RuleWalker { + + visitSourceFile(sourceFile) { + utils.forEachComment(sourceFile, (fullText, commentRange) => { + let isTodoComment = fullText.substring(commentRange.pos, commentRange.end).includes('TODO'); + + if (commentRange.kind === ts.SyntaxKind.MultiLineCommentTrivia && isTodoComment) { + this.addFailureAt(commentRange.pos, commentRange.end - commentRange.pos, ERROR_MESSAGE); + } + }); + } +} + +exports.Rule = Rule; \ No newline at end of file diff --git a/tslint.json b/tslint.json index cf9ba7cf9a57..37fb35232b24 100644 --- a/tslint.json +++ b/tslint.json @@ -1,5 +1,8 @@ { - "rulesDirectory": ["node_modules/tslint-no-unused-var"], + "rulesDirectory": [ + "./node_modules/tslint-no-unused-var/", + "./tools/tslint-rules/" + ], "rules": { "max-line-length": [true, 100], // Disable this flag because of SHA tslint#48b0c597f9257712c7d1f04b55ed0aa60e333f6a @@ -26,6 +29,7 @@ "no-unused-expression": true, "no-unused-var": [true, {"ignore-pattern": "^(_.*)$"}], "no-var-keyword": true, + "no-exposed-todo": true, "no-debugger": true, "one-line": [ true, From 0b7347c6da3b4ba9a67977cb49eb8d808adb0091 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 12 Apr 2017 14:30:14 -0700 Subject: [PATCH 2/2] Address feedback --- tools/tslint-rules/noExposedTodoRule.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/tslint-rules/noExposedTodoRule.js b/tools/tslint-rules/noExposedTodoRule.js index b67f5dbc4fd0..c92eb9a4adb8 100644 --- a/tools/tslint-rules/noExposedTodoRule.js +++ b/tools/tslint-rules/noExposedTodoRule.js @@ -2,7 +2,9 @@ const ts = require('typescript'); const utils = require('tsutils'); const Lint = require('tslint'); -const ERROR_MESSAGE = 'Todo inside of the comment might be published in the docs.'; +const ERROR_MESSAGE = + 'A TODO may only appear in inline (//) style comments. ' + + 'This is meant to prevent a TODO from being accidentally included in any public API docs.'; /** * Rule that walks through all comments inside of the library and adds failures when it