From a0d944b41b3f99d3432d90e8fdb62c58db1aff4e Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Sat, 30 Mar 2019 11:06:46 +0100
Subject: [PATCH] build: add lint rule to catch SimpleChanges direct property
 access

Adds a lint rule to help catch cases like #15206 until we can get something going through tsetse. Note this is only scoped to `ngOnChanges` and accesses of the `SimpleChanges` objects since that's where we've been having issues.
---
 .../ngOnChangesPropertyAccessRule.ts          | 53 +++++++++++++++++++
 tslint.json                                   |  1 +
 2 files changed, 54 insertions(+)
 create mode 100644 tools/tslint-rules/ngOnChangesPropertyAccessRule.ts

diff --git a/tools/tslint-rules/ngOnChangesPropertyAccessRule.ts b/tools/tslint-rules/ngOnChangesPropertyAccessRule.ts
new file mode 100644
index 000000000000..caa05fe3e3bf
--- /dev/null
+++ b/tools/tslint-rules/ngOnChangesPropertyAccessRule.ts
@@ -0,0 +1,53 @@
+import * as ts from 'typescript';
+import * as Lint from 'tslint';
+import * as tsutils from 'tsutils';
+
+/**
+ * Rule that catches cases where a property of a `SimpleChanges` object is accessed directly,
+ * rather than through a literal. Accessing properties of `SimpleChanges` directly can break
+ * when using Closure's property renaming.
+ */
+export class Rule extends Lint.Rules.TypedRule {
+  applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
+    return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program));
+  }
+}
+
+class Walker extends Lint.ProgramAwareRuleWalker {
+  visitMethodDeclaration(method: ts.MethodDeclaration) {
+    // Walk through all of the `ngOnChanges` methods that have at least one parameter.
+    if (method.name.getText() !== 'ngOnChanges' || !method.parameters.length || !method.body) {
+      return;
+    }
+
+    const walkChildren = (node: ts.Node) => {
+      // Walk through all the nodes and look for property access expressions
+      // (e.g. `changes.something`). Note that this is different from element access
+      // expressions which look like `changes['something']`.
+      if (tsutils.isPropertyAccessExpression(node)) {
+        const symbol = this.getTypeChecker().getTypeAtLocation(node.expression).symbol;
+
+        // Add a failure if we're trying to access a property on a SimpleChanges object
+        // directly, because it can cause issues with Closure's property renaming.
+        if (symbol && symbol.name === 'SimpleChanges') {
+          const expressionName = node.expression.getText();
+          const propName = node.name.getText();
+
+          this.addFailureAtNode(node, 'Accessing properties of SimpleChanges objects directly ' +
+                                      'is not allowed. Use index access instead (e.g. ' +
+                                      `${expressionName}.${propName} should be ` +
+                                      `${expressionName}['${propName}']).`);
+        }
+      }
+
+      // Don't walk the property accesses inside of call expressions. This prevents us
+      // from flagging cases like `changes.hasOwnProperty('something')` incorrectly.
+      if (!tsutils.isCallExpression(node)) {
+        node.forEachChild(walkChildren);
+      }
+    };
+
+    method.body.forEachChild(walkChildren);
+    super.visitMethodDeclaration(method);
+  }
+}
diff --git a/tslint.json b/tslint.json
index df986e17930a..ac5318dfec24 100644
--- a/tslint.json
+++ b/tslint.json
@@ -99,6 +99,7 @@
     "no-import-spacing": true,
     "no-private-getters": true,
     "setters-after-getters": true,
+    "ng-on-changes-property-access": true,
     "rxjs-imports": true,
     "require-breaking-change-version": true,
     "no-host-decorator-in-concrete": [