Move the tsickle auto-quoting logic into a tsetse check.
This check is only useful for code that would be opmitized through a property
renaming optimizer (like Closure, and experiemental uglifyJS option, AFAIK).
It is intentionally, exposed but not turned on by default for rules_typescript.
PiperOrigin-RevId: 236218830
diff --git a/docs/property-renaming-safe.md b/docs/property-renaming-safe.md
new file mode 100644
index 0000000..716b0d9
--- /dev/null
+++ b/docs/property-renaming-safe.md
@@ -0,0 +1,43 @@
+<!-- FIXME(alexeagle): generate the docs from the sources -->
+
+# Property renaming safe
+
+This check will reject code patterns that are likely to not be safe for an
+optimization known as "property renaming". Closure compiler is currently one of
+the main JS optimizer that performs that optimization and anecdotally it results
+in 5-10% smaller bundles (see [more
+info](http://closuretools.blogspot.com/2011/01/property-by-any-other-name-part-1.html)).
+
+Property renaming is the optimization where writes and accesses to object
+properties like `x.userName` are rewritten to a shorter name like `x.a`. It
+works great until other code expects the property to be the specific string
+'userName'. This check will help catch bad optimization errors like this earlier
+(during TS compilation).
+
+The check will reject a property access on an object through an index signature
+in its type. For example, if you write this:
+
+ interface IdxSig {
+ [key: string]: string;
+ }
+
+ function propAccess(x: IdxSig) {
+ x.prop; // new error
+ x['prop']; // ok
+ }
+
+
+the compiler will start erring like this:
+
+ .../errors.ts:6:5 - error TS21227: Property prop is not declared on Type IdxSig.
+ The type has a string index signature, but it is being accessed using a dotted
+ property access.
+
+ 6 x.prop; // new error
+
+
+The presence of an index signature in the type most often means that the
+properties of the object that carries that type are created outside the static
+property renaming scope - for example JSON.parse, or dynamic for-in loop. Thus
+accessing them without a quoted index signature will result in an incorrect
+optimization.
diff --git a/internal/tsetse/error_code.ts b/internal/tsetse/error_code.ts
index 22da857..1e0ea3a 100644
--- a/internal/tsetse/error_code.ts
+++ b/internal/tsetse/error_code.ts
@@ -1,7 +1,15 @@
+/**
+ * Error codes for tsetse checks.
+ *
+ * Start with 21222 and increase linearly.
+ * The intent is for these codes to be fixed, so that tsetse users can
+ * search for them in user forums and other media.
+ */
export enum ErrorCode {
CHECK_RETURN_VALUE = 21222,
EQUALS_NAN = 21223,
BAN_EXPECT_TRUTHY_PROMISE = 21224,
MUST_USE_PROMISES = 21225,
BAN_PROMISE_AS_CONDITION = 21226,
+ PROPERTY_RENAMING_SAFE = 21227,
}
diff --git a/internal/tsetse/rules/property_renaming_safe.ts b/internal/tsetse/rules/property_renaming_safe.ts
new file mode 100644
index 0000000..7b84244
--- /dev/null
+++ b/internal/tsetse/rules/property_renaming_safe.ts
@@ -0,0 +1,54 @@
+
+import * as ts from 'typescript';
+
+import {Checker} from '../checker';
+import {ErrorCode} from '../error_code';
+import {AbstractRule} from '../rule';
+
+/**
+ * A Tsetse rule that checks for some potential unsafe property renaming
+ * patterns.
+ *
+ * Note: This rule can have false positives.
+ */
+export class Rule extends AbstractRule {
+ readonly ruleName = 'property-renaming-safe';
+ readonly code = ErrorCode.PROPERTY_RENAMING_SAFE;
+
+ register(checker: Checker) {
+ checker.on(
+ ts.SyntaxKind.PropertyAccessExpression,
+ checkIndexSignAccessedWithPropAccess, this.code);
+ }
+}
+
+// Copied from tsickle/src/quoting_transformer.ts, with the intention of
+// removing it from there and only keeping a tsetse rule about this.
+function checkIndexSignAccessedWithPropAccess(
+ checker: Checker, pae: ts.PropertyAccessExpression) {
+ // Reject dotted accesses to types that have an index type declared to quoted
+ // accesses, to avoid Closure renaming one access but not the other. This can
+ // happen because TS allows dotted access to string index types.
+ const typeChecker = checker.typeChecker;
+ const t = typeChecker.getTypeAtLocation(pae.expression);
+ if (!t.getStringIndexType()) return;
+ // Types can have string index signatures and declared properties (of the
+ // matching type). These properties have a symbol, as opposed to pure string
+ // index types.
+ const propSym = typeChecker.getSymbolAtLocation(pae.name);
+ // The decision to return below is a judgement call. Presumably, in most
+ // situations, dotted access to a property is correct, and should not be
+ // turned into quoted access even if there is a string index on the type.
+ // However it is possible to construct programs where this is incorrect, e.g.
+ // where user code assigns into a property through the index access in another
+ // location.
+ if (propSym) return;
+
+ checker.addFailureAtNode(
+ pae.name,
+ `Property ${pae.name.text} is not declared on Type ` +
+ `${typeChecker.typeToString(t)}. The type has a string index ` +
+ `signature, but it is being accessed using a dotted property ` +
+ `access.
+See http://tsetse.info/property-renaming-safe.`);
+}
diff --git a/internal/tsetse/tests/property_renaming_safe/errors.ts b/internal/tsetse/tests/property_renaming_safe/errors.ts
new file mode 100644
index 0000000..b4ed844
--- /dev/null
+++ b/internal/tsetse/tests/property_renaming_safe/errors.ts
@@ -0,0 +1,64 @@
+/* tslint:disable */
+interface IdxSig {
+ [key: string]: string;
+}
+
+function propAccess(x: IdxSig) {
+ x.prop; // error
+ x['prop']; // ok
+}
+
+function descructuring(x: IdxSig) {
+ const {prop} = x; // ok, but should be an error.
+}
+
+interface MixedIdxSig extends IdxSig {
+ namedProp: string;
+}
+
+function mixedPropAccess(x: MixedIdxSig) {
+ x.namedProp; // ok
+ x['namedProp']; // ok
+ x.prop; // error
+ x['prop']; // ok
+}
+
+function genericAccess<T extends IdxSig>(x: T) {
+ x.prop; // error
+ x['prop']; // ok
+}
+
+interface MixedIndexSigUsedInUnion {
+ [key: string]: string;
+ namedProp2: string;
+}
+
+function unionType(x: MixedIdxSig|MixedIndexSigUsedInUnion) {
+ x.prop; // error
+ x['prop']; // ok
+}
+
+/**
+ * Curiously Record<string, T> is treated like an index signature.
+ */
+function recordStringType(x: Record<string, number>) {
+ x.prop; // error
+ x['prop']; // ok
+}
+
+/**
+ * But narrowing the generic parameter to a string literal union exempts it.
+ */
+function recordNarrowType(x: Record<'prop'|'other', number>) {
+ x.prop; // ok
+ x['prop']; // ok
+}
+
+/**
+ * Similary, to Records mapped types of of 'in string' are threated like a
+ * string index signature.
+ */
+function mappedType(x: {[x in string]: boolean}) {
+ x.prop; // error
+ x['prop']; // ok
+}
