Ensure whitespaces after semicolon in structural dir #356
Conversation
wKoza
changed the title
src/angularWhitespaceRule.ts
Outdated
| @@ -12,6 +12,8 @@ const InterpolationClose = Config.interpolation[1]; | |||
| const InterpolationNoWhitespaceRe =new RegExp(`${InterpolationOpen}\\S(.*?)\\S${InterpolationClose}|${InterpolationOpen}\\s(.*?)\\S${InterpolationClose}|${InterpolationOpen}\\S(.*?)\\s${InterpolationClose}`); | |||
| const InterpolationExtraWhitespaceRe = | |||
| new RegExp(`${InterpolationOpen}\\s\\s(.*?)\\s${InterpolationClose}|${InterpolationOpen}\\s(.*?)\\s\\s${InterpolationClose}`); | |||
| const SemicolonNoWhitespaceRe =new RegExp(/;\S(.*)/); | |||
src/angularWhitespaceRule.ts
Outdated
| const getSemicolonReplacements = (text: ast.BoundDirectivePropertyAst, absolutePosition: number) => { | ||
|
|
||
| return [ | ||
| new Lint.Replacement(absolutePosition, 1, "; ") |
src/angularWhitespaceRule.ts
Outdated
| if (SemicolonNoWhitespaceRe.test(expr)) { | ||
| error = 'Missing whitespace after semicolon; expecting \'; expr\''; | ||
| const internalStart = expr.search(SemicolonNoWhitespaceRe)+1; | ||
| const start = prop.sourceSpan.start.offset + internalStart + directive.length - directive.split("=")[1].trim().length +1; |
There was a problem hiding this comment.
Lets use single quotes and add space after the +.
src/angularWhitespaceRule.ts
Outdated
| error = 'Missing whitespace after semicolon; expecting \'; expr\''; | ||
| const internalStart = expr.search(SemicolonNoWhitespaceRe)+1; | ||
| const start = prop.sourceSpan.start.offset + internalStart + directive.length - directive.split("=")[1].trim().length +1; | ||
| const absolutePosition = context.getSourcePosition(start-1); |
|
@wKoza awesome job man! I left some very minor style-related comments. Will provide further feedback after AngularUP. |
|
hey @wKoza, nice job... I found one case which should be still probably taken in account: template: The arguments of the pipe should not been checked as it could influence an output of that pipe. Other question is if you shouldn't check for whitespace specifically. Now it is possible to have |
src/angularWhitespaceRule.ts
Outdated
|
|
||
| if (SemicolonNoWhitespaceRe.test(expr)) { | ||
| error = 'Missing whitespace after semicolon; expecting \'; expr\''; | ||
| const internalStart = expr.search(SemicolonNoWhitespaceRe)+1; |
There was a problem hiding this comment.
what about adding
"whitespace": [
true,
"check-branch",
"check-decl",
"check-module",
"check-operator",
"check-preblock",
"check-separator",
"check-type"
],to tslint.json and not have a necessity to deal with this in CR? :)
btw. tslint is outdated and some rules are already gone. If you want, I can prepare some PR with proposal.
There was a problem hiding this comment.
@rokerkony yes, it's best if we add the checks as part of CI.
A PR will be good.
src/angularWhitespaceRule.ts
Outdated
| if (prop.sourceSpan) { | ||
| const directive = (<any>prop.sourceSpan).toString(); | ||
| let expr = directive.split("=")[1].trim(); | ||
| expr = expr.substring(1,expr.length-1).trim(); |
src/angularWhitespaceRule.ts
Outdated
|
|
||
| if (prop.sourceSpan) { | ||
| const directive = (<any>prop.sourceSpan).toString(); | ||
| let expr = directive.split("=")[1].trim(); |
|
I suppose we can generalize that we should not check string literals. |
…hitespaceAfterSemiColon
closes #330