This repository has been archived by the owner on Mar 25, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 889
Add rule: strict-string-expressions #4807
Merged
JoshuaKGoldberg
merged 27 commits into
palantir:master
from
ColCh:4512-strict-string-expressions
Aug 12, 2019
Merged
Changes from 23 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
570a5d4
feat: add strict-string-expressions
ColCh 928d5f7
docs(strictStringExpressionsRule): add option desc
ColCh fda5ea4
feat(strictStringExpressionsRule): add fixer
ColCh f7ddea6
test: add strict-string-expressions into all.ts
ColCh b2f9bc5
type: remove declare
ColCh e8a809a
refactor(strictStringExpressionsRule): eliminate copy-paste
ColCh 5a9e7da
style: fix tslint errors
ColCh fa07d78
fix(strictStringExpressionsRule): exclude any type from checks
ColCh fe6332d
style: fix sytle erorrs
ColCh 643268c
refactor: rename function name to more cohesive
ColCh 2fafc18
refactor(strictStringExpressionsRule): add more renaimings
ColCh 217229f
style(strictStringExpressionsRule): prettify
ColCh 235d7aa
fix: do not require numbers to be stringified
ColCh 589f06f
test(strictStringExpressions): add string and number literals
ColCh 6259432
test(strictStringExpressionsRule): fix nit
ColCh 88a2278
test(strictStringExpressionsRule): add string uni in test
ColCh 7d8253d
test: add typeof window case
ColCh ae1c2be
feat: handle union type
ColCh 7de511b
feat(strictStringExpressionsRule): add allow-empty-types option
ColCh bb841e0
feat: allo empty types in all.ts config
ColCh 2c9603e
style(strictStringExpressionsRule): fix lint rules
ColCh cf3f90d
feat(strictStringExpressionsRule): allow booleans
ColCh 6ee97af
style(linter): fix codestyle
ColCh c460673
refactor(strictStringExpressionsRule): rename private helpers
ColCh b99df8d
docs(strictStringExpressionsRule): update license year
ColCh 2ac3f27
refactor(strictStringExpressionsRule): make allow-empty-types to appear
ColCh bb95c45
test(stringStrictExpressions): correct test
ColCh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
/** | ||
* @license | ||
* Copyright 2018 Palantir Technologies, Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import { isTypeFlagSet, isUnionType } from "tsutils"; | ||
import * as ts from "typescript"; | ||
|
||
import * as Lint from "../index"; | ||
|
||
const OPTION_ALLOW_EMPTY_TYPES = "allow-empty-types"; | ||
|
||
interface Options { | ||
[OPTION_ALLOW_EMPTY_TYPES]?: boolean; | ||
} | ||
|
||
export class Rule extends Lint.Rules.TypedRule { | ||
public static CONVERSION_REQUIRED = "Explicit conversion to string type required"; | ||
|
||
public static metadata: Lint.IRuleMetadata = { | ||
description: "Disable implicit toString() calls", | ||
descriptionDetails: Lint.Utils.dedent` | ||
Require explicit toString() call for variables used in strings. By default only strings are allowed. | ||
|
||
The following nodes are checked: | ||
|
||
* String literals ("foo" + bar) | ||
* ES6 templates (\`foo \${bar}\`)`, | ||
hasFix: true, | ||
optionExamples: [ | ||
true, | ||
[ | ||
true, | ||
{ | ||
[OPTION_ALLOW_EMPTY_TYPES]: true, | ||
}, | ||
], | ||
], | ||
options: { | ||
properties: { | ||
[OPTION_ALLOW_EMPTY_TYPES]: { | ||
type: "boolean", | ||
}, | ||
}, | ||
type: "object", | ||
}, | ||
optionsDescription: Lint.Utils.dedent` | ||
Following arguments may be optionally provided: | ||
* \`${OPTION_ALLOW_EMPTY_TYPES}\` allows \`null\`, \`undefined\` and \`never\` to be passed into strings without explicit conversion`, | ||
requiresTypeInfo: true, | ||
ruleName: "strict-string-expressions", | ||
type: "functionality", | ||
typescriptOnly: true, | ||
}; | ||
|
||
public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { | ||
return this.applyWithFunction( | ||
sourceFile, | ||
walk, | ||
this.getRuleOptions(), | ||
program.getTypeChecker(), | ||
); | ||
} | ||
|
||
private getRuleOptions(): Options { | ||
if (this.ruleArguments[0] === undefined) { | ||
return {}; | ||
} else { | ||
return this.ruleArguments[0] as Options; | ||
} | ||
} | ||
} | ||
|
||
function walk(ctx: Lint.WalkContext<Options>, checker: ts.TypeChecker): void { | ||
const { sourceFile, options } = ctx; | ||
ts.forEachChild(sourceFile, function cb(node: ts.Node): void { | ||
switch (node.kind) { | ||
case ts.SyntaxKind.BinaryExpression: { | ||
ColCh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const binaryExpr = node as ts.BinaryExpression; | ||
if (binaryExpr.operatorToken.kind === ts.SyntaxKind.PlusToken) { | ||
const leftIsPassedAsIs = isTypeConvertsToStringEasily( | ||
checker.getTypeAtLocation(binaryExpr.left), | ||
options, | ||
); | ||
const rightIsPassedAsIs = isTypeConvertsToStringEasily( | ||
checker.getTypeAtLocation(binaryExpr.right), | ||
options, | ||
); | ||
const leftIsFailed = !leftIsPassedAsIs && rightIsPassedAsIs; | ||
const rightIsFailed = leftIsPassedAsIs && !rightIsPassedAsIs; | ||
if (leftIsFailed || rightIsFailed) { | ||
const expression = leftIsFailed ? binaryExpr.left : binaryExpr.right; | ||
addFailure(binaryExpr, expression); | ||
} | ||
} | ||
break; | ||
} | ||
case ts.SyntaxKind.TemplateSpan: { | ||
const templateSpanNode = node as ts.TemplateSpan; | ||
const type = checker.getTypeAtLocation(templateSpanNode.expression); | ||
const shouldPassAsIs = isTypeConvertsToStringEasily(type, options); | ||
if (!shouldPassAsIs) { | ||
const { expression } = templateSpanNode; | ||
addFailure(templateSpanNode, expression); | ||
} | ||
} | ||
} | ||
return ts.forEachChild(node, cb); | ||
}); | ||
|
||
function addFailure(node: ts.Node, expression: ts.Expression) { | ||
const fix = Lint.Replacement.replaceFromTo( | ||
expression.getStart(), | ||
expression.end, | ||
`String(${expression.getText()})`, | ||
); | ||
ctx.addFailureAtNode(node, Rule.CONVERSION_REQUIRED, fix); | ||
} | ||
} | ||
|
||
const isEmptyType = (type: ts.Type): boolean => | ||
isTypeFlagSet(type, ts.TypeFlags.Null) || | ||
isTypeFlagSet(type, ts.TypeFlags.VoidLike) || | ||
isTypeFlagSet(type, ts.TypeFlags.Undefined) || | ||
isTypeFlagSet(type, ts.TypeFlags.Never); | ||
|
||
function isTypeConvertsToStringEasily(type: ts.Type, options: Options): boolean { | ||
if (isUnionType(type)) { | ||
return type.types.every(unionAtomicType => | ||
isTypeConvertsToStringEasily(unionAtomicType, options), | ||
); | ||
} | ||
|
||
if (options[OPTION_ALLOW_EMPTY_TYPES] && isEmptyType(type)) { | ||
return true; | ||
} | ||
|
||
return ( | ||
isTypeFlagSet(type, ts.TypeFlags.BooleanLike) || | ||
isTypeFlagSet(type, ts.TypeFlags.StringOrNumberLiteral) || | ||
isTypeFlagSet(type, ts.TypeFlags.NumberLike) || | ||
isTypeFlagSet(type, ts.TypeFlags.StringLike) || | ||
isTypeFlagSet(type, ts.TypeFlags.Any) | ||
); | ||
} |
133 changes: 133 additions & 0 deletions
133
test/rules/strict-string-expressions/allow-empty-types/test.ts.fix
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
const fooAny: any; | ||
const fooStr: string = 'foo'; | ||
const fooNumber = 2; | ||
class FooClass {} | ||
class ClassWithToString { | ||
public static toString () { return ''; } | ||
public toString () { return ''; } | ||
} | ||
const classWithToString = new ClassWithToString(); | ||
const FooStr = new String('foo'); | ||
const fooArr = ['foo']; | ||
const emptyArr = []; | ||
const stringUni = "foo" | "bar"; | ||
const booleanVar: boolean; | ||
|
||
`foo` | ||
`${'str literal'}` | ||
`${123}` | ||
`${booleanVar}` | ||
`${fooAny}` | ||
`${fooStr}` | ||
`${stringUni}` | ||
`${fooNumber}` | ||
`${(typeof window)}` | ||
`${String(FooClass)}` | ||
`${String(ClassWithToString)}` | ||
`${String(classWithToString)}` | ||
`${String(FooStr)}` | ||
`${String(fooArr)}` | ||
`${String(emptyArr)}` | ||
|
||
`${String('str literal')}` | ||
`${String(123)}` | ||
`${String(booleanVar)}` | ||
`${String(fooAny)}` | ||
`${String(fooStr)}` | ||
`${String(stringUni)}` | ||
`${String(fooNumber)}` | ||
`${String((typeof window))}` | ||
`${String(FooClass)}` | ||
`${String(ClassWithToString)}` | ||
`${String(classWithToString)}` | ||
`${String(FooStr)}` | ||
`${String(fooArr)}` | ||
`${String(emptyArr)}` | ||
|
||
`${'str literal'.toString()}` | ||
`${123..toString()}` | ||
`${booleanVar.toString()}` | ||
`${fooAny.toString()}` | ||
`${fooStr.toString()}` | ||
`${stringUni.toString()}` | ||
`${fooNumber.toString()}` | ||
`${(typeof window).toString()}` | ||
`${FooClass.toString()}` | ||
`${ClassWithToString.toString()}` | ||
`${classWithToString.toString()}` | ||
`${FooStr.toString()}` | ||
`${fooArr.toString()}` | ||
`${emptyArr.toString()}` | ||
|
||
'str' + 'str literal' + 'str' | ||
'str' + 123 + 'str' | ||
'str' + booleanVar + 'str' | ||
'str' + fooAny + 'str' | ||
'str' + fooStr + 'str' | ||
'str' + stringUni + 'str' | ||
'str' + fooNumber + 'str' | ||
'str' + (typeof window) + 'str' | ||
'str' + String(FooClass) + 'str' | ||
'str' + String(ClassWithToString) + 'str' | ||
'str' + String(classWithToString) + 'str' | ||
'str' + String(FooStr) + 'str' | ||
'str' + String(fooArr) + 'str' | ||
'str' + String(emptyArr) + 'str' | ||
|
||
'str' + String('str literal') + 'str' | ||
'str' + String(123) + 'str' | ||
'str' + String(booleanVar) + 'str' | ||
'str' + String(fooAny) + 'str' | ||
'str' + String(fooStr) + 'str' | ||
'str' + String(stringUni) + 'str' | ||
'str' + String(fooNumber) + 'str' | ||
'str' + String((typeof window)) + 'str' | ||
'str' + String(FooClass) + 'str' | ||
'str' + String(ClassWithToString) + 'str' | ||
'str' + String(classWithToString) + 'str' | ||
'str' + String(FooStr) + 'str' | ||
'str' + String(fooArr) + 'str' | ||
'str' + String(emptyArr) + 'str' | ||
|
||
'str' + 'str literal'.toString() + 'str' | ||
'str' + 123..toString() + 'str' | ||
'str' + booleanVar.toString() + 'str' | ||
'str' + fooAny.toString() + 'str' | ||
'str' + fooStr.toString() + 'str' | ||
'str' + stringUni.toString() + 'str' | ||
'str' + fooNumber.toString() + 'str' | ||
'str' + (typeof window).toString() + 'str' | ||
'str' + FooClass.toString() + 'str' | ||
'str' + ClassWithToString.toString() + 'str' | ||
'str' + classWithToString.toString() + 'str' | ||
'str' + FooStr.toString() + 'str' | ||
'str' + fooArr.toString() + 'str' | ||
'str' + emptyArr.toString() + 'str' | ||
|
||
const barFooStrOrUndef: string | undefined; | ||
const barFooStrOrNull: string | null; | ||
const neverType: never; | ||
|
||
`${barFooStrOrUndef}` | ||
`${barFooStrOrNull}` | ||
`${neverType}` | ||
|
||
`${String(barFooStrOrUndef)}` | ||
`${String(barFooStrOrNull)}` | ||
`${String(neverType)}` | ||
|
||
`${barFooStrOrUndef.toString()}` | ||
`${barFooStrOrNull.toString()}` | ||
`${neverType.toString()}` | ||
|
||
'str' + barFooStrOrUndef + 'str' | ||
'str' + barFooStrOrNull + 'str' | ||
'str' + neverType + 'str' | ||
|
||
'str' + String(barFooStrOrUndef) + 'str' | ||
'str' + String(barFooStrOrNull) + 'str' | ||
'str' + String(neverType) + 'str' | ||
|
||
'str' + barFooStrOrUndef.toString() + 'str' | ||
'str' + barFooStrOrNull.toString() + 'str' | ||
'str' + neverType.toString() + 'str' |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
real case here 🎉