[new-fixer] file-header rule #3475
Conversation
c93378a to
ca86965
Compare
src/rules/fileHeaderRule.ts
Outdated
| const { pos, end, kind } = commentDetails; | ||
| const commentText = text.substring(pos, kind === ts.SyntaxKind.SingleLineCommentTrivia ? end : end - 2); | ||
| if (!headerFormat.test(commentText)) { | ||
| const isFirstCommentAHeader = ONLY_WHITESPACE.test(text.substring(offset, pos)); |
There was a problem hiding this comment.
the substring could contain line breaks.The regexp probably needs the m flag to make this work as expected.
src/rules/fileHeaderRule.ts
Outdated
| const isFirstCommentAHeader = ONLY_WHITESPACE.test(text.substring(offset, pos)); | ||
| const fix = textToInsert !== undefined | ||
| ? isFirstCommentAHeader | ||
| ? Lint.Replacement.replaceFromTo(pos, end, this.createComment(sourceFile, textToInsert, 0)) |
There was a problem hiding this comment.
Is removing the existing comment really expected? It could contain JSDoc or something similar useful.
There was a problem hiding this comment.
Hmm, it's a tricky call. If it's a malformed header, you'd want it to be removed, otherwise you probably wouldn't want it to be. I don't think there's any way to automatically determine which is the case, so we'll want to just pick the option that is most correct in most cases.
I think I agree with your thoughts that replacing the header might be unexpected more often than desired. (Plus it'll be simpler code w/o that feature.) I guess we could make the behavior configurable in the future if desired
src/rules/fileHeaderRule.ts
Outdated
| const lineEnding = `${maybeCarriageReturn}\n`; | ||
| return [ | ||
| "/*", | ||
| ...commentText.split(lineEnding).map((line) => ` * ${line}`), |
There was a problem hiding this comment.
consider splitting at /\r?\n/g. That allows users to only use \n in their config while using \r\n in source files.
|
Updated to no longer replace the first comment it finds. I also modified the insertion code a little bit to handle insert newlines in (hopefully) the most expected manner. |
|
@adidahiya , are you expecting to include this in the next version? Do you already have a release date for that? |
PR checklist
Overview of change:
Adds a fixer to
file-header. The rule will now insert text from a new second option as the header of your file if that option is provided and--fixis enabled.If the rule finds a header comment that's incorrect, it's replaced with the specified header. If the rule doesn't find a header comment at all, the specified header is inserted at the top of the document.
Is there anything you'd like reviewers to focus on?
Is the newline handling legit?
CHANGELOG.md entry:
[new-fixer]
file-header