From de1992353a0f236f2af869bb1051d98cbfdd5892 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 20 Apr 2017 23:26:07 +0200 Subject: [PATCH 1/6] jsdoc-format: rewrite and fix [bugfix] `jsdoc-format`: fixed error position if line ends with `\r\n` --- src/rules/jsdocFormatRule.ts | 93 +++++++++++++++--------------------- 1 file changed, 38 insertions(+), 55 deletions(-) diff --git a/src/rules/jsdocFormatRule.ts b/src/rules/jsdocFormatRule.ts index 22c5a904916..829673696cc 100644 --- a/src/rules/jsdocFormatRule.ts +++ b/src/rules/jsdocFormatRule.ts @@ -45,68 +45,51 @@ export class Rule extends Lint.Rules.AbstractRule { public static FORMAT_FAILURE_STRING = "jsdoc is not formatted correctly on this line"; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new JsdocWalker(sourceFile, this.getOptions())); + return this.applyWithFunction(sourceFile, walk); } } -class JsdocWalker extends Lint.RuleWalker { - public visitSourceFile(node: ts.SourceFile) { - utils.forEachComment(node, (fullText, comment) => { - if (comment.kind === ts.SyntaxKind.MultiLineCommentTrivia) { - this.findFailuresForJsdocComment(fullText.substring(comment.pos, comment.end), comment.pos); - } - }); - } - - private findFailuresForJsdocComment(commentText: string, startingPosition: number) { - const currentPosition = startingPosition; - // the file may be different depending on the OS it was originally authored on - // can't rely on require('os').EOL or process.platform as that is the execution env - // regex is: split optionally on \r\n, but alwasy split on \n if no \r exists - const lines = commentText.split(/\r?\n/); +function walk(ctx: Lint.WalkContext) { + return utils.forEachComment(ctx.sourceFile, (fullText, {kind, pos, end}) => { + if (kind !== ts.SyntaxKind.MultiLineCommentTrivia || fullText[pos + 2] !== "*" || fullText[pos + 3] === "*") { + return; + } + const lines = fullText.slice(pos + 3, end - 2).split(/\n/); const firstLine = lines[0]; - let jsdocPosition = currentPosition; - - // regex is: start of string, followed by any amount of whitespace, followed by /** but not more than 2 ** - const isJsdocMatch = firstLine.match(/^\s*\/\*\*([^*]|$)/); - if (isJsdocMatch != null) { - if (lines.length === 1) { - const firstLineMatch = firstLine.match(/^\s*\/\*\* (.* )?\*\/$/); - if (firstLineMatch == null) { - this.addFailureAt(jsdocPosition, firstLine.length, Rule.FORMAT_FAILURE_STRING); - } - return; - } - - const indexToMatch = firstLine.indexOf("**") + this.getLineAndCharacterOfPosition(currentPosition).character; - // all lines but the first and last - const otherLines = lines.splice(1, lines.length - 2); - jsdocPosition += firstLine.length + 1; // + 1 for the splitted-out newline - for (const line of otherLines) { - // regex is: start of string, followed by any amount of whitespace, followed by *, - // followed by either a space or the end of the string - const asteriskMatch = line.match(/^\s*\*( |$)/); - if (asteriskMatch == null) { - this.addFailureAt(jsdocPosition, line.length, Rule.FORMAT_FAILURE_STRING); - } - const asteriskIndex = line.indexOf("*"); - if (asteriskIndex !== indexToMatch) { - this.addFailureAt(jsdocPosition, line.length, Rule.ALIGNMENT_FAILURE_STRING); - } - jsdocPosition += line.length + 1; // + 1 for the splitted-out newline + if (lines.length === 1) { + if (!firstLine.startsWith(" ") || !firstLine.endsWith(" ")) { + ctx.addFailure(pos, end, Rule.FORMAT_FAILURE_STRING); } + return; + } - const lastLine = lines[lines.length - 1]; - // regex is: start of string, followed by any amount of whitespace, followed by */, - // followed by the end of the string - const endBlockCommentMatch = lastLine.match(/^\s*\*\/$/); - if (endBlockCommentMatch == null) { - this.addFailureAt(jsdocPosition, lastLine.length, Rule.FORMAT_FAILURE_STRING); + const alignColumn = ts.getLineAndCharacterOfPosition(ctx.sourceFile, pos + 1).character; + let lineStart = pos + firstLine.length + 4; + const endIndex = lines.length - 1; + for (let i = 1; i < endIndex; ++i) { + const line = lines[i]; + // regex is: start of string, followed by any amount of whitespace, followed by *, + // followed by either a space or the end of the string + if (!/^\s*\*(?: |\r?$)/.test(line)) { + ctx.addFailureAt(lineStart, lineLength(line), Rule.FORMAT_FAILURE_STRING); } - const lastAsteriskIndex = lastLine.indexOf("*"); - if (lastAsteriskIndex !== indexToMatch) { - this.addFailureAt(jsdocPosition, lastLine.length, Rule.ALIGNMENT_FAILURE_STRING); + if (line.indexOf("*") !== alignColumn) { + ctx.addFailureAt(lineStart, lineLength(line), Rule.ALIGNMENT_FAILURE_STRING); } + lineStart += line.length + 1; // + 1 for the splitted-out newline } - } + const lastLine = lines[endIndex]; + // last line should only consist of whitespace + if (lastLine.search(/\S/) !== -1) { + ctx.addFailure(lineStart, end, Rule.FORMAT_FAILURE_STRING); + } + if (lastLine.length !== alignColumn) { + ctx.addFailure(lineStart, end, Rule.ALIGNMENT_FAILURE_STRING); + } + + }); +} + +function lineLength(line: string) { + return line.endsWith("\r") ? line.length - 1 : line.length; } From 4dd95e73f8ba8a3deb301bbd3a8f2b637c86503d Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 20 Apr 2017 23:31:42 +0200 Subject: [PATCH 2/6] add comment --- src/rules/jsdocFormatRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/jsdocFormatRule.ts b/src/rules/jsdocFormatRule.ts index 829673696cc..4602bf9fcc9 100644 --- a/src/rules/jsdocFormatRule.ts +++ b/src/rules/jsdocFormatRule.ts @@ -64,7 +64,7 @@ function walk(ctx: Lint.WalkContext) { } const alignColumn = ts.getLineAndCharacterOfPosition(ctx.sourceFile, pos + 1).character; - let lineStart = pos + firstLine.length + 4; + let lineStart = pos + firstLine.length + 4; // +3 for the comment start "/**" and +1 for the newline const endIndex = lines.length - 1; for (let i = 1; i < endIndex; ++i) { const line = lines[i]; From 0c9a9ccac97344f809188f19f5ca0f35f30d0599 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Fri, 21 Apr 2017 09:55:43 +0200 Subject: [PATCH 3/6] handle empty comment --- src/rules/jsdocFormatRule.ts | 3 ++- test/rules/jsdoc-format/jsdoc.ts.lint | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/rules/jsdocFormatRule.ts b/src/rules/jsdocFormatRule.ts index 4602bf9fcc9..f7c0daa70a5 100644 --- a/src/rules/jsdocFormatRule.ts +++ b/src/rules/jsdocFormatRule.ts @@ -51,7 +51,8 @@ export class Rule extends Lint.Rules.AbstractRule { function walk(ctx: Lint.WalkContext) { return utils.forEachComment(ctx.sourceFile, (fullText, {kind, pos, end}) => { - if (kind !== ts.SyntaxKind.MultiLineCommentTrivia || fullText[pos + 2] !== "*" || fullText[pos + 3] === "*") { + if (kind !== ts.SyntaxKind.MultiLineCommentTrivia || + fullText[pos + 2] !== "*" || fullText[pos + 3] === "*" || fullText[pos + 3] === "/") { return; } const lines = fullText.slice(pos + 3, end - 2).split(/\n/); diff --git a/test/rules/jsdoc-format/jsdoc.ts.lint b/test/rules/jsdoc-format/jsdoc.ts.lint index e5bfe71884f..448be1be18a 100644 --- a/test/rules/jsdoc-format/jsdoc.ts.lint +++ b/test/rules/jsdoc-format/jsdoc.ts.lint @@ -68,6 +68,8 @@ one * /** a good one */ + /**/ // no jsdoc + } // Regression test: jsdoc rule shouldn't look inside template strings (https://github.com/palantir/tslint/issues/332) From a0ab45534566eaa8b2377b55f4ae36440cc38d60 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Fri, 21 Apr 2017 10:04:23 +0200 Subject: [PATCH 4/6] Fix tests with crlf --- .gitattributes | 1 + test/rules/jsdoc-format/jsdoc-windows.ts.lint | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/.gitattributes b/.gitattributes index f141c2ac31b..0734f3c7a7f 100644 --- a/.gitattributes +++ b/.gitattributes @@ -6,3 +6,4 @@ *.fix text eol=lf /test/rules/linebreak-style/**/CRLF/*.lint text eol=crlf /test/rules/linebreak-style/**/LF/*.fix text eol=crlf +/test/rules/jsdoc-format/**/jsdoc-windows.ts.lint text eol=crlf diff --git a/test/rules/jsdoc-format/jsdoc-windows.ts.lint b/test/rules/jsdoc-format/jsdoc-windows.ts.lint index ffe987ccb6c..5d6090b9b0a 100644 --- a/test/rules/jsdoc-format/jsdoc-windows.ts.lint +++ b/test/rules/jsdoc-format/jsdoc-windows.ts.lint @@ -7,4 +7,18 @@ */ class MyClass { -} \ No newline at end of file +} + +/** + *here comes some invalid jsdoc +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [format] + to ensure that the error is shown in the right place +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [format] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [asterisks] + * +*/ +~~ [asterisks] +function foo() {} + +[format]: jsdoc is not formatted correctly on this line +[asterisks]: asterisks in jsdoc must be aligned \ No newline at end of file From 6238b222299f9e8e354bbcb5d9d1327fbd42ee53 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Fri, 21 Apr 2017 10:16:38 +0200 Subject: [PATCH 5/6] simplify handling of \r --- src/rules/jsdocFormatRule.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/rules/jsdocFormatRule.ts b/src/rules/jsdocFormatRule.ts index f7c0daa70a5..1bb431a699d 100644 --- a/src/rules/jsdocFormatRule.ts +++ b/src/rules/jsdocFormatRule.ts @@ -55,7 +55,7 @@ function walk(ctx: Lint.WalkContext) { fullText[pos + 2] !== "*" || fullText[pos + 3] === "*" || fullText[pos + 3] === "/") { return; } - const lines = fullText.slice(pos + 3, end - 2).split(/\n/); + const lines = fullText.slice(pos + 3, end - 2).split("\n"); const firstLine = lines[0]; if (lines.length === 1) { if (!firstLine.startsWith(" ") || !firstLine.endsWith(" ")) { @@ -68,16 +68,16 @@ function walk(ctx: Lint.WalkContext) { let lineStart = pos + firstLine.length + 4; // +3 for the comment start "/**" and +1 for the newline const endIndex = lines.length - 1; for (let i = 1; i < endIndex; ++i) { - const line = lines[i]; + const line = lines[i].endsWith("\r") ? lines[i].slice(0, -1) : lines[i]; // regex is: start of string, followed by any amount of whitespace, followed by *, // followed by either a space or the end of the string - if (!/^\s*\*(?: |\r?$)/.test(line)) { - ctx.addFailureAt(lineStart, lineLength(line), Rule.FORMAT_FAILURE_STRING); + if (!/^\s*\*(?: |$)/.test(line)) { + ctx.addFailureAt(lineStart, line.length, Rule.FORMAT_FAILURE_STRING); } if (line.indexOf("*") !== alignColumn) { - ctx.addFailureAt(lineStart, lineLength(line), Rule.ALIGNMENT_FAILURE_STRING); + ctx.addFailureAt(lineStart, line.length, Rule.ALIGNMENT_FAILURE_STRING); } - lineStart += line.length + 1; // + 1 for the splitted-out newline + lineStart += lines[i].length + 1; // + 1 for the splitted-out newline } const lastLine = lines[endIndex]; // last line should only consist of whitespace @@ -90,7 +90,3 @@ function walk(ctx: Lint.WalkContext) { }); } - -function lineLength(line: string) { - return line.endsWith("\r") ? line.length - 1 : line.length; -} From 69009e050dfd172cf5183ff18cae85a130a67252 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Fri, 21 Apr 2017 10:44:56 +0200 Subject: [PATCH 6/6] handle files with BOM --- src/rules/jsdocFormatRule.ts | 12 ++++++++++-- test/rules/jsdoc-format/jsdoc-bom.ts.lint | 3 +++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 test/rules/jsdoc-format/jsdoc-bom.ts.lint diff --git a/src/rules/jsdocFormatRule.ts b/src/rules/jsdocFormatRule.ts index 1bb431a699d..1a6fecd6fa3 100644 --- a/src/rules/jsdocFormatRule.ts +++ b/src/rules/jsdocFormatRule.ts @@ -58,13 +58,13 @@ function walk(ctx: Lint.WalkContext) { const lines = fullText.slice(pos + 3, end - 2).split("\n"); const firstLine = lines[0]; if (lines.length === 1) { - if (!firstLine.startsWith(" ") || !firstLine.endsWith(" ")) { + if (firstLine[0] !== " " || !firstLine.endsWith(" ")) { ctx.addFailure(pos, end, Rule.FORMAT_FAILURE_STRING); } return; } - const alignColumn = ts.getLineAndCharacterOfPosition(ctx.sourceFile, pos + 1).character; + const alignColumn = getAlignColumn(ctx.sourceFile, pos + 1); let lineStart = pos + firstLine.length + 4; // +3 for the comment start "/**" and +1 for the newline const endIndex = lines.length - 1; for (let i = 1; i < endIndex; ++i) { @@ -90,3 +90,11 @@ function walk(ctx: Lint.WalkContext) { }); } + +function getAlignColumn(sourceFile: ts.SourceFile, pos: number) { + const result = ts.getLineAndCharacterOfPosition(sourceFile, pos); + // handle files starting with BOM + return result.line === 0 && sourceFile.text[0] === "\uFEFF" + ? result.character - 1 + : result.character; +} diff --git a/test/rules/jsdoc-format/jsdoc-bom.ts.lint b/test/rules/jsdoc-format/jsdoc-bom.ts.lint new file mode 100644 index 00000000000..7cbf9e07419 --- /dev/null +++ b/test/rules/jsdoc-format/jsdoc-bom.ts.lint @@ -0,0 +1,3 @@ +/** + * This is a valid jsdoc comment and should show no warning even though there is a BOM in the first line + */