From 9aeff89e397a0ac6c565349f6ee10403ed419b32 Mon Sep 17 00:00:00 2001 From: Juliano Date: Mon, 17 Jul 2017 15:10:35 -0300 Subject: [PATCH 1/6] build: tslint rule to enforce html tags escaping in comments - Address @willshowell's comments --- tools/tslint-rules/noUnscapedHtmlTagRule.js | 67 +++++++++++++++++++++ tslint.json | 7 ++- 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 tools/tslint-rules/noUnscapedHtmlTagRule.js diff --git a/tools/tslint-rules/noUnscapedHtmlTagRule.js b/tools/tslint-rules/noUnscapedHtmlTagRule.js new file mode 100644 index 000000000000..eff701fd40a1 --- /dev/null +++ b/tools/tslint-rules/noUnscapedHtmlTagRule.js @@ -0,0 +1,67 @@ +const ts = require('typescript'); +const utils = require('tsutils'); +const Lint = require('tslint'); + +const ERROR_MESSAGE = + 'An HTML tag may only appear in a JSDoc comment if it is escaped.' + + ' This prevents failures in document generation caused by a misinterpreted tag.'; + +/** + * Rule that walks through all comments inside of the library and adds failures when it + * detects unescaped HTML tags inside of multi-line comments. + */ +class Rule extends Lint.Rules.AbstractRule { + + apply(sourceFile) { + return this.applyWithWalker(new NoUnescapedHtmlTagWalker(sourceFile, this.getOptions())); + } +} + +class NoUnescapedHtmlTagWalker extends Lint.RuleWalker { + + visitSourceFile(sourceFile) { + utils.forEachComment(sourceFile, (fullText, commentRange) => { + + let isEscapedHtmlTag = true; + const matches = new RegExp(/[<>]/); + + const numberOfBackticks = fullText.split('`').length - 1; + if ((numberOfBackticks === 1) || ((numberOfBackticks === 0) && matches.test(fullText))) { + isEscapedHtmlTag = false; + } + + // if there are no backticks and [<>], there's no need for any more checks + if ((numberOfBackticks > 1) && matches.test(fullText)) { + // if there are backticks there should be an even number of them + if (!!(numberOfBackticks % 2)) { + isEscapedHtmlTag = false; + } else { + /** + * This logic behaves like a stack structure. isBacktickWithoutMatch plays + * the stack role: it is set to true whenever, during a comment scan, an 'open' + * backtick is found in the string, and it is set back to false when the next matching + * (closing) backtick is found. Every backtick must have a matching. < and > + * must always be between two matching backticks. + */ + const splitedFullText = fullText.split(''); + let isBacktickWithoutMatch = false; + + for (var i = 0; i < splitedFullText.length; i++) { + if (splitedFullText[i] === '`') { + isBacktickWithoutMatch = !isBacktickWithoutMatch; + } else if (matches.test(splitedFullText[i]) && !isBacktickWithoutMatch) { + isEscapedHtmlTag = false; + break; + } + } + } + } + + if (commentRange.kind === ts.SyntaxKind.MultiLineCommentTrivia && !isEscapedHtmlTag) { + this.addFailureAt(commentRange.pos, commentRange.end - commentRange.pos, ERROR_MESSAGE); + } + }); + } +} + +exports.Rule = Rule; diff --git a/tslint.json b/tslint.json index 779e990be37d..3c5bc5ba333f 100644 --- a/tslint.json +++ b/tslint.json @@ -28,6 +28,7 @@ "no-unused-expression": true, "no-var-keyword": true, "member-access": [true, "no-public"], + "no-unescaped-html-tag": true, "no-debugger": true, "no-unused-variable": true, "one-line": [ @@ -79,7 +80,11 @@ {"name": "Object.assign", "message": "Use the spread operator instead."} ], // Disallows importing the whole RxJS library. Submodules can be still imported. - "import-blacklist": [true, "rxjs", "rxjs/operators"], + "import-blacklist": [ + true, + "rxjs", + "rxjs/operators" + ], // Avoids inconsistent linebreak styles in source files. Forces developers to use LF linebreaks. "linebreak-style": [true, "LF"], // Namespaces are no allowed, because of Closure compiler. From 09688ba2c771d099bd3e43a4b5b17ffbb52a99ea Mon Sep 17 00:00:00 2001 From: Juliano Date: Thu, 2 Nov 2017 17:39:33 -0200 Subject: [PATCH 2/6] address comments --- tools/tslint-rules/noUnscapedHtmlTagRule.js | 63 +++++++++++---------- tslint.json | 6 +- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/tools/tslint-rules/noUnscapedHtmlTagRule.js b/tools/tslint-rules/noUnscapedHtmlTagRule.js index eff701fd40a1..c4eb0b3c121a 100644 --- a/tools/tslint-rules/noUnscapedHtmlTagRule.js +++ b/tools/tslint-rules/noUnscapedHtmlTagRule.js @@ -22,45 +22,46 @@ class NoUnescapedHtmlTagWalker extends Lint.RuleWalker { visitSourceFile(sourceFile) { utils.forEachComment(sourceFile, (fullText, commentRange) => { - let isEscapedHtmlTag = true; - const matches = new RegExp(/[<>]/); + const htmlIsEscaped = parseForHtml(fullText); - const numberOfBackticks = fullText.split('`').length - 1; - if ((numberOfBackticks === 1) || ((numberOfBackticks === 0) && matches.test(fullText))) { - isEscapedHtmlTag = false; + if (commentRange.kind === ts.SyntaxKind.MultiLineCommentTrivia && !htmlIsEscaped) { + this.addFailureAt(commentRange.pos, commentRange.end - commentRange.pos, ERROR_MESSAGE); } + }); + } + + /** Gets whether the comment's HTML, if any, is properly escaped */ + parseForHtml(fullText) { + const matches = new RegExp(/[<>]/); + + const backtickCount = fullText.split('`').length - 1; + if ((backtickCount === 1) || ((backtickCount === 0) && matches.test(fullText))) { + return false; + } - // if there are no backticks and [<>], there's no need for any more checks - if ((numberOfBackticks > 1) && matches.test(fullText)) { - // if there are backticks there should be an even number of them - if (!!(numberOfBackticks % 2)) { - isEscapedHtmlTag = false; - } else { - /** - * This logic behaves like a stack structure. isBacktickWithoutMatch plays - * the stack role: it is set to true whenever, during a comment scan, an 'open' - * backtick is found in the string, and it is set back to false when the next matching - * (closing) backtick is found. Every backtick must have a matching. < and > - * must always be between two matching backticks. - */ - const splitedFullText = fullText.split(''); - let isBacktickWithoutMatch = false; + // if there are no backticks and no [<>], there's no need for any more checks + if ((backtickCount > 1) && matches.test(fullText)) { + // if there are backticks there should be an even number of them + if (backtickCount % 2) { + return false; + } else { + // < and > must always be between two matching backticks. + const fullTextArray = fullText.split(''); - for (var i = 0; i < splitedFullText.length; i++) { - if (splitedFullText[i] === '`') { - isBacktickWithoutMatch = !isBacktickWithoutMatch; - } else if (matches.test(splitedFullText[i]) && !isBacktickWithoutMatch) { - isEscapedHtmlTag = false; - break; - } + // Whether an opening backtick has been found without a closing pair + let openBacktick = false; + + for (let i = 0; i < fullTextArray.length; i++) { + if (fullTextArray[i] === '`') { + openBacktick = !openBacktick; + } else if (matches.test(fullTextArray[i]) && !openBacktick) { + return false; } } } + } - if (commentRange.kind === ts.SyntaxKind.MultiLineCommentTrivia && !isEscapedHtmlTag) { - this.addFailureAt(commentRange.pos, commentRange.end - commentRange.pos, ERROR_MESSAGE); - } - }); + return true; } } diff --git a/tslint.json b/tslint.json index 3c5bc5ba333f..0547cbfa25d1 100644 --- a/tslint.json +++ b/tslint.json @@ -80,11 +80,7 @@ {"name": "Object.assign", "message": "Use the spread operator instead."} ], // Disallows importing the whole RxJS library. Submodules can be still imported. - "import-blacklist": [ - true, - "rxjs", - "rxjs/operators" - ], + "import-blacklist": [true, "rxjs", "rxjs/operators"], // Avoids inconsistent linebreak styles in source files. Forces developers to use LF linebreaks. "linebreak-style": [true, "LF"], // Namespaces are no allowed, because of Closure compiler. From 02dda8d1138e4cf846039424f2a53d56d33930c7 Mon Sep 17 00:00:00 2001 From: Juliano Date: Thu, 2 Nov 2017 19:01:22 -0200 Subject: [PATCH 3/6] tightening the logic --- tools/tslint-rules/noUnscapedHtmlTagRule.js | 36 ++++++++++----------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/tools/tslint-rules/noUnscapedHtmlTagRule.js b/tools/tslint-rules/noUnscapedHtmlTagRule.js index c4eb0b3c121a..f9945f355120 100644 --- a/tools/tslint-rules/noUnscapedHtmlTagRule.js +++ b/tools/tslint-rules/noUnscapedHtmlTagRule.js @@ -33,31 +33,29 @@ class NoUnescapedHtmlTagWalker extends Lint.RuleWalker { /** Gets whether the comment's HTML, if any, is properly escaped */ parseForHtml(fullText) { const matches = new RegExp(/[<>]/); - const backtickCount = fullText.split('`').length - 1; - if ((backtickCount === 1) || ((backtickCount === 0) && matches.test(fullText))) { + + // An odd number of backticks or html without backticks is invalid + if ((backtickCount % 2) || ((backtickCount === 0) && matches.test(fullText))) { return false; } - // if there are no backticks and no [<>], there's no need for any more checks - if ((backtickCount > 1) && matches.test(fullText)) { - // if there are backticks there should be an even number of them - if (backtickCount % 2) { - return false; - } else { - // < and > must always be between two matching backticks. - const fullTextArray = fullText.split(''); + // Text without html is valid + if (!matches.test(fullText)) { + return true; + } + + // < and > must always be between two matching backticks. + const fullTextArray = fullText.split(''); - // Whether an opening backtick has been found without a closing pair - let openBacktick = false; + // Whether an opening backtick has been found without a closing pair + let openBacktick = false; - for (let i = 0; i < fullTextArray.length; i++) { - if (fullTextArray[i] === '`') { - openBacktick = !openBacktick; - } else if (matches.test(fullTextArray[i]) && !openBacktick) { - return false; - } - } + for (let i = 0; i < fullTextArray.length; i++) { + if (fullTextArray[i] === '`') { + openBacktick = !openBacktick; + } else if (matches.test(fullTextArray[i]) && !openBacktick) { + return false; } } From 7cd8cff07c2e111369f9b11e5c5136bf35b8f566 Mon Sep 17 00:00:00 2001 From: Juliano Date: Fri, 3 Nov 2017 06:55:17 -0200 Subject: [PATCH 4/6] address @rafaelss95's comments --- tools/tslint-rules/noUnscapedHtmlTagRule.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/tslint-rules/noUnscapedHtmlTagRule.js b/tools/tslint-rules/noUnscapedHtmlTagRule.js index f9945f355120..78cb55cff7cb 100644 --- a/tools/tslint-rules/noUnscapedHtmlTagRule.js +++ b/tools/tslint-rules/noUnscapedHtmlTagRule.js @@ -32,11 +32,11 @@ class NoUnescapedHtmlTagWalker extends Lint.RuleWalker { /** Gets whether the comment's HTML, if any, is properly escaped */ parseForHtml(fullText) { - const matches = new RegExp(/[<>]/); + const matches = /[<>]/; const backtickCount = fullText.split('`').length - 1; // An odd number of backticks or html without backticks is invalid - if ((backtickCount % 2) || ((backtickCount === 0) && matches.test(fullText))) { + if (backtickCount % 2 || (!backtickCount && matches.test(fullText))) { return false; } @@ -46,15 +46,14 @@ class NoUnescapedHtmlTagWalker extends Lint.RuleWalker { } // < and > must always be between two matching backticks. - const fullTextArray = fullText.split(''); - + // Whether an opening backtick has been found without a closing pair let openBacktick = false; - for (let i = 0; i < fullTextArray.length; i++) { - if (fullTextArray[i] === '`') { + for (const char of fullText) { + if (char === '`') { openBacktick = !openBacktick; - } else if (matches.test(fullTextArray[i]) && !openBacktick) { + } else if (matches.test(char) && !openBacktick) { return false; } } From 1b03d22f93d5224ee7ec90a182d027729cdf0f0e Mon Sep 17 00:00:00 2001 From: Juliano Date: Fri, 3 Nov 2017 11:03:03 -0200 Subject: [PATCH 5/6] switch to typescript --- ...mlTagRule.js => noUnescapedHtmlTagRule.ts} | 27 +++++++++---------- tslint.json | 6 ++--- 2 files changed, 16 insertions(+), 17 deletions(-) rename tools/tslint-rules/{noUnscapedHtmlTagRule.js => noUnescapedHtmlTagRule.ts} (74%) diff --git a/tools/tslint-rules/noUnscapedHtmlTagRule.js b/tools/tslint-rules/noUnescapedHtmlTagRule.ts similarity index 74% rename from tools/tslint-rules/noUnscapedHtmlTagRule.js rename to tools/tslint-rules/noUnescapedHtmlTagRule.ts index 78cb55cff7cb..1d35f8757ca9 100644 --- a/tools/tslint-rules/noUnscapedHtmlTagRule.js +++ b/tools/tslint-rules/noUnescapedHtmlTagRule.ts @@ -1,37 +1,38 @@ -const ts = require('typescript'); -const utils = require('tsutils'); -const Lint = require('tslint'); +import * as ts from 'typescript'; +import * as Lint from 'tslint'; +import * as utils from 'tsutils'; const ERROR_MESSAGE = - 'An HTML tag may only appear in a JSDoc comment if it is escaped.' + + 'An HTML tag delimiter (< or >) may only appear in a JSDoc comment if it is escaped.' + ' This prevents failures in document generation caused by a misinterpreted tag.'; /** * Rule that walks through all comments inside of the library and adds failures when it * detects unescaped HTML tags inside of multi-line comments. */ -class Rule extends Lint.Rules.AbstractRule { +export class Rule extends Lint.Rules.AbstractRule { - apply(sourceFile) { + apply(sourceFile: ts.SourceFile) { return this.applyWithWalker(new NoUnescapedHtmlTagWalker(sourceFile, this.getOptions())); } } class NoUnescapedHtmlTagWalker extends Lint.RuleWalker { - visitSourceFile(sourceFile) { + visitSourceFile(sourceFile: ts.SourceFile) { utils.forEachComment(sourceFile, (fullText, commentRange) => { - - const htmlIsEscaped = parseForHtml(fullText); - + const htmlIsEscaped = + this._parseForHtml(fullText.substring(commentRange.pos, commentRange.end)); if (commentRange.kind === ts.SyntaxKind.MultiLineCommentTrivia && !htmlIsEscaped) { this.addFailureAt(commentRange.pos, commentRange.end - commentRange.pos, ERROR_MESSAGE); } }); + + super.visitSourceFile(sourceFile); } /** Gets whether the comment's HTML, if any, is properly escaped */ - parseForHtml(fullText) { + private _parseForHtml(fullText: string): boolean { const matches = /[<>]/; const backtickCount = fullText.split('`').length - 1; @@ -46,7 +47,7 @@ class NoUnescapedHtmlTagWalker extends Lint.RuleWalker { } // < and > must always be between two matching backticks. - + // Whether an opening backtick has been found without a closing pair let openBacktick = false; @@ -61,5 +62,3 @@ class NoUnescapedHtmlTagWalker extends Lint.RuleWalker { return true; } } - -exports.Rule = Rule; diff --git a/tslint.json b/tslint.json index 0547cbfa25d1..203183e2a75f 100644 --- a/tslint.json +++ b/tslint.json @@ -28,7 +28,6 @@ "no-unused-expression": true, "no-var-keyword": true, "member-access": [true, "no-public"], - "no-unescaped-html-tag": true, "no-debugger": true, "no-unused-variable": true, "one-line": [ @@ -119,7 +118,8 @@ "missing-rollup-globals": [ true, "./tools/package-tools/rollup-globals.ts", - "src/+(lib|cdk|material-examples|material-experimental|cdk-experimental)/**/*.ts" - ] + "src/+(lib|cdk|material-examples)/**/*.ts" + ], + "no-unescaped-html-tag": true } } From 03b541f17013dd881128ad59781db404a2c6ac4c Mon Sep 17 00:00:00 2001 From: Juliano Date: Wed, 24 Jan 2018 19:53:10 -0200 Subject: [PATCH 6/6] bring back material-experimental|cdk-experimental --- tslint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tslint.json b/tslint.json index 203183e2a75f..1621579d9375 100644 --- a/tslint.json +++ b/tslint.json @@ -118,7 +118,7 @@ "missing-rollup-globals": [ true, "./tools/package-tools/rollup-globals.ts", - "src/+(lib|cdk|material-examples)/**/*.ts" + "src/+(lib|cdk|material-examples|material-experimental|cdk-experimental)/**/*.ts" ], "no-unescaped-html-tag": true }