Adds a tsetse rule to prevent constructing Sets using a string.
Disallows `new Set('abc')` since that results in a Set containing 'a', 'b' and 'c'.
- If that really is the intention of the code, it can be converted to `new Set('abc' as Iterable<string>)`
- If the intention was to have a Set containing 'abc', it should be `new Set(['abc'])`
PiperOrigin-RevId: 290914197
diff --git a/internal/tsetse/error_code.ts b/internal/tsetse/error_code.ts
index 87b7fd2..10d36c5 100644
--- a/internal/tsetse/error_code.ts
+++ b/internal/tsetse/error_code.ts
@@ -14,4 +14,5 @@
PROPERTY_RENAMING_SAFE = 21227,
CONFORMANCE_PATTERN = 21228,
BAN_MUTABLE_EXPORTS = 21229,
+ BAN_STRING_INITIALIZED_SETS = 21230,
}
diff --git a/internal/tsetse/rules/ban_string_initialized_sets_rule.ts b/internal/tsetse/rules/ban_string_initialized_sets_rule.ts
new file mode 100644
index 0000000..44087ba
--- /dev/null
+++ b/internal/tsetse/rules/ban_string_initialized_sets_rule.ts
@@ -0,0 +1,63 @@
+/**
+ * @fileoverview Bans `new Set(<string>)` since it is a potential source of bugs
+ * due to strings also implementing `Iterable<string>`.
+ */
+
+import * as ts from 'typescript';
+
+import {Checker} from '../checker';
+import {ErrorCode} from '../error_code';
+import {AbstractRule} from '../rule';
+
+const errorMsg = 'Value passed to Set constructor is a string. This will' +
+ ' create a Set of the characters of the string, rather than a Set' +
+ ' containing the string. To make a Set of the string, pass an array' +
+ ' containing the string. To make a Set of the characters, use \'as\' to ' +
+ ' create an Iterable<string>, eg: new Set(myStr as Iterable<string>).';
+
+export class Rule extends AbstractRule {
+ readonly ruleName = 'ban-string-initialized-sets';
+ readonly code = ErrorCode.BAN_STRING_INITIALIZED_SETS;
+
+ register(checker: Checker) {
+ checker.on(ts.SyntaxKind.NewExpression, checkNewExpression, this.code);
+ }
+}
+
+function checkNewExpression(checker: Checker, node: ts.NewExpression) {
+ const typeChecker = checker.typeChecker;
+
+ // Check that it's a Set which is being constructed
+ const ctorTypeSymbol =
+ typeChecker.getTypeAtLocation(node.expression).getSymbol();
+
+ if (!ctorTypeSymbol || ctorTypeSymbol.getEscapedName() !== 'SetConstructor') {
+ return;
+ }
+ const isES2015SetCtor = ctorTypeSymbol.declarations.some((decl) => {
+ return sourceFileIsStdLib(decl.getSourceFile());
+ });
+ if (!isES2015SetCtor) return;
+
+ // If there's no arguments provided, then it's not a string so bail out.
+ if (!node.arguments || node.arguments.length !== 1) return;
+
+ // Check the type of the first argument, expanding union & intersection types
+ const arg = node.arguments[0];
+ const argType = typeChecker.getTypeAtLocation(arg);
+ const allTypes = argType.isUnionOrIntersection() ? argType.types : [argType];
+
+ // Checks if the type (or any of the union/intersection types) are either
+ // strings or string literals.
+ const typeContainsString = allTypes.some((tsType) => {
+ return (tsType.getFlags() & ts.TypeFlags.StringLike) !== 0;
+ });
+
+ if (!typeContainsString) return;
+
+ checker.addFailureAtNode(arg, errorMsg);
+}
+
+function sourceFileIsStdLib(sourceFile: ts.SourceFile) {
+ return /lib\.es2015\.(collection|iterable)\.d\.ts$/.test(sourceFile.fileName);
+}
diff --git a/internal/tsetse/runner.ts b/internal/tsetse/runner.ts
index a2b76d4..8e2926a 100644
--- a/internal/tsetse/runner.ts
+++ b/internal/tsetse/runner.ts
@@ -11,6 +11,7 @@
import {AbstractRule} from './rule';
import {Rule as BanExpectTruthyPromiseRule} from './rules/ban_expect_truthy_promise_rule';
import {Rule as BanPromiseAsConditionRule} from './rules/ban_promise_as_condition_rule';
+import {Rule as BanStringInitializedSetsRule} from './rules/ban_string_initialized_sets_rule';
import {Rule as CheckReturnValueRule} from './rules/check_return_value_rule';
import {Rule as EqualsNanRule} from './rules/equals_nan_rule';
import {Rule as MustUsePromisesRule} from './rules/must_use_promises_rule';
@@ -25,6 +26,7 @@
new BanExpectTruthyPromiseRule(),
new MustUsePromisesRule(),
new BanPromiseAsConditionRule(),
+ new BanStringInitializedSetsRule(),
];
/**
diff --git a/internal/tsetse/tests/ban_string_initialized_sets/negatives.ts b/internal/tsetse/tests/ban_string_initialized_sets/negatives.ts
new file mode 100644
index 0000000..e9a188a
--- /dev/null
+++ b/internal/tsetse/tests/ban_string_initialized_sets/negatives.ts
@@ -0,0 +1,42 @@
+// tslint:disable
+function emptySet() {
+ const set = new Set();
+}
+
+function noConstructorArgs() {
+ const set = new Set;
+}
+
+function nonStringSet() {
+ const set = new Set([1, 2, 3]);
+}
+
+// This is an allowable way to create a set of strings
+function setOfStrings() {
+ const set = new Set(['abc']);
+}
+
+function setOfChars() {
+ const set = new Set('abc'.split(''));
+}
+
+function explicitlyAllowString() {
+ const set = new Set('abc' as Iterable<string>);
+}
+
+// checks that just a property called 'Set' doesn't trigger the error
+function justAKeyCalledSet(obj: {Set: {new (s: string): any}}) {
+ const set = new obj.Set('abc');
+}
+
+function destructuredConstructorCalledSet(obj: {Set: {new (s: string): any}}) {
+ const {Set} = obj;
+ const set = new Set('abc');
+}
+
+function locallyDeclaredSet() {
+ class Set {
+ constructor(private s: string) {}
+ }
+ const set = new Set('abc');
+}
diff --git a/internal/tsetse/tests/ban_string_initialized_sets/positives.ts b/internal/tsetse/tests/ban_string_initialized_sets/positives.ts
new file mode 100644
index 0000000..a4ec695
--- /dev/null
+++ b/internal/tsetse/tests/ban_string_initialized_sets/positives.ts
@@ -0,0 +1,51 @@
+// tslint:disable
+function setWithStringLiteral() {
+ const set = new Set('abc');
+}
+
+function setWithStringVariable(s: string) {
+ const set = new Set(s);
+}
+
+function setWithStringUnionType(s: string|string[]) {
+ const set = new Set(s);
+}
+
+function setWithStringExpression(fn: () => string) {
+ const set = new Set(fn());
+}
+
+function setWithStringExpression2() {
+ const set = new Set(Math.random() < 0.5 ? 'a' : 'b');
+}
+
+type TypeA = string|Set<string>;
+type TypeB = TypeA|(Iterable<string>&IterableIterator<string>);
+function setWithComplexInitializationType(s: TypeB) {
+ const set = new Set(s);
+}
+
+function setWithUnionStringType(s: string&{toString(): string}) {
+ const set = new Set(s);
+}
+
+function setWithLocalAlias() {
+ const TotallyNotASet = Set;
+ const set = new TotallyNotASet('abc');
+}
+
+function setWithMultipleAliases() {
+ const Foo = Set;
+ const Bar = Foo;
+ const Baz = Bar;
+ const set = new Baz('abc');
+}
+
+function setUsingSetConstructorType(ctor: SetConstructor) {
+ const set = new ctor('abc');
+}
+
+type MySet = SetConstructor;
+function setUsingAliasedSetConstructor(ctor: MySet) {
+ const set = new ctor('abc');
+}