refactor: annotate remaining base classes with @directive for… (#17279)

* build: add lint rule to ensure classes with angular features are decorated

Read more about this here: https://hackmd.io/vuQfavzfRG6KUCtU7oK_EA.

* refactor: annotate base classes with @directive for ivy.

These base classes define class members with Angular features. Therefore
they need to be decorated with `@Directive()` so that the classes can be
extended in Ivy.

This is an official migration that will be performed for CLI
applications in version 9. angular/angular#32590.

Author: devversion

src/cdk-experimental/popover-edit/popover-edit.spec.ts

@@ -3,7 +3,7 @@ import {LEFT_ARROW, UP_ARROW, RIGHT_ARROW, DOWN_ARROW, TAB} from '@angular/cdk/k
 import {CdkTableModule} from '@angular/cdk/table';
 import {dispatchKeyboardEvent} from '@angular/cdk/testing';
 import {CommonModule} from '@angular/common';
-import {Component, ElementRef, Type, ViewChild} from '@angular/core';
+import {Component, Directive, ElementRef, Type, ViewChild} from '@angular/core';
 import {ComponentFixture, fakeAsync, flush, TestBed, tick, inject} from '@angular/core/testing';
 import {FormsModule, NgForm} from '@angular/forms';
 import {BidiModule, Direction} from '@angular/cdk/bidi';
@@ -61,6 +61,10 @@ interface PeriodicElement {
   weight: number;
 }
 
+@Directive({
+  // TODO(devversion): this selector can be removed when we update to Angular 9.0.
+  selector: 'do-not-use-abstract-cdk-popover-edit-base-test-component'
+})
 abstract class BaseTestComponent {
   @ViewChild('table', {static: false}) table: ElementRef;
 

src/material-experimental/mdc-button/button-base.ts

@@ -7,7 +7,7 @@
  */
 
 import {Platform} from '@angular/cdk/platform';
-import {ElementRef, NgZone, ViewChild} from '@angular/core';
+import {Directive, ElementRef, Inject, NgZone, Optional, ViewChild} from '@angular/core';
 import {
   CanColor,
   CanColorCtor,
@@ -21,6 +21,7 @@ import {
   mixinDisableRipple,
   RippleAnimationConfig
 } from '@angular/material/core';
+import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
 import {numbers} from '@material/ripple';
 
 /** Inputs common to all buttons. */
@@ -78,6 +79,10 @@ export const _MatButtonBaseMixin: CanDisableRippleCtor&CanDisableCtor&CanColorCt
     typeof MatButtonMixinCore = mixinColor(mixinDisabled(mixinDisableRipple(MatButtonMixinCore)));
 
 /** Base class for all buttons.  */
+@Directive({
+  // TODO(devversion): this selector can be removed when we update to Angular 9.0.
+  selector: 'do-not-use-abstract-mat-button-base'
+})
 export class MatButtonBase extends _MatButtonBaseMixin implements CanDisable, CanColor,
                                                                   CanDisableRipple {
   /** The ripple animation configuration to use for the buttons. */
@@ -94,7 +99,8 @@ export class MatButtonBase extends _MatButtonBaseMixin implements CanDisable, Ca
 
   constructor(
       elementRef: ElementRef, public _platform: Platform, public _ngZone: NgZone,
-      public _animationMode?: string) {
+      // TODO(devversion): Injection can be removed if angular/angular#32981 is fixed.
+      @Optional() @Inject(ANIMATION_MODULE_TYPE) public _animationMode?: string) {
     super(elementRef);
 
     // For each of the variant selectors that is present in the button's host
@@ -144,10 +150,16 @@ export const MAT_ANCHOR_HOST = {
 /**
  * Anchor button base.
  */
+@Directive({
+  // TODO(devversion): this selector can be removed when we update to Angular 9.0.
+  selector: 'do-not-use-abstract-mat-anchor-base'
+})
 export class MatAnchorBase extends MatButtonBase {
   tabIndex: number;
 
-  constructor(elementRef: ElementRef, platform: Platform, ngZone: NgZone, animationMode?: string) {
+  constructor(elementRef: ElementRef, platform: Platform, ngZone: NgZone,
+              // TODO(devversion): Injection can be removed if angular/angular#32981 is fixed.
+              @Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string) {
     super(elementRef, platform, ngZone, animationMode);
   }
 

src/material-experimental/mdc-button/module.ts

@@ -10,6 +10,7 @@ import {CommonModule} from '@angular/common';
 import {NgModule} from '@angular/core';
 import {MatCommonModule, MatRippleModule} from '@angular/material/core';
 import {MatAnchor, MatButton} from './button';
+import {MatAnchorBase, MatButtonBase} from './button-base';
 import {MatFabAnchor, MatFabButton} from './fab';
 import {MatIconAnchor, MatIconButton} from './icon-button';
 
@@ -31,6 +32,10 @@ import {MatIconAnchor, MatIconButton} from './icon-button';
     MatIconButton,
     MatFabAnchor,
     MatFabButton,
+    // TODO(devversion): remove when `MatButtonBase` becomes a selectorless Directive.
+    MatButtonBase,
+    // TODO(devversion): remove when `MatAnchorBase` becomes a selectorless Directive.
+    MatAnchorBase,
   ],
 })
 export class MatButtonModule {

src/material-experimental/popover-edit/popover-edit.spec.ts

@@ -3,7 +3,7 @@ import {LEFT_ARROW, UP_ARROW, RIGHT_ARROW, DOWN_ARROW, TAB} from '@angular/cdk/k
 import {MatTableModule} from '@angular/material/table';
 import {dispatchKeyboardEvent} from '@angular/cdk/testing';
 import {CommonModule} from '@angular/common';
-import {Component, ElementRef, Type, ViewChild} from '@angular/core';
+import {Component, Directive, ElementRef, Type, ViewChild} from '@angular/core';
 import {ComponentFixture, fakeAsync, flush, TestBed, tick, inject} from '@angular/core/testing';
 import {FormsModule, NgForm} from '@angular/forms';
 import {OverlayContainer} from '@angular/cdk/overlay';
@@ -59,6 +59,10 @@ interface PeriodicElement {
   weight: number;
 }
 
+@Directive({
+  // TODO(devversion): this selector can be removed when we update to Angular 9.0.
+  selector: 'do-not-use-abstract-mat-popover-edit-base-test-component'
+})
 abstract class BaseTestComponent {
   @ViewChild('table', {static: false}) table: ElementRef;
 

src/material/menu/menu-module.ts

@@ -11,7 +11,7 @@ import {CommonModule} from '@angular/common';
 import {NgModule} from '@angular/core';
 import {MatCommonModule, MatRippleModule} from '@angular/material/core';
 import {MatMenuContent} from './menu-content';
-import {_MatMenu} from './menu';
+import {_MatMenu, _MatMenuBase, MatMenu} from './menu';
 import {MatMenuItem} from './menu-item';
 import {
   MatMenuTrigger,
@@ -24,7 +24,14 @@ import {
  */
 @NgModule({
   exports: [MatMenuTrigger, MatMenuContent, MatCommonModule],
-  declarations: [MatMenuTrigger, MatMenuContent],
+  declarations: [
+    MatMenuTrigger,
+    MatMenuContent,
+    // TODO(devversion): remove when `MatMenu` becomes a selectorless Directive.
+    MatMenu,
+    // TODO(devversion): remove when `_MatMenuBase` becomes a selectorless Directive.
+    _MatMenuBase
+  ],
   providers: [MAT_MENU_SCROLL_STRATEGY_FACTORY_PROVIDER]
 })
 // tslint:disable-next-line:class-name

src/material/menu/menu.ts

@@ -25,6 +25,7 @@ import {
   Component,
   ContentChild,
   ContentChildren,
+  Directive,
   ElementRef,
   EventEmitter,
   Inject,
@@ -90,6 +91,10 @@ export function MAT_MENU_DEFAULT_OPTIONS_FACTORY(): MatMenuDefaultOptions {
 const MAT_MENU_BASE_ELEVATION = 4;
 
 /** Base class with all of the `MatMenu` functionality. */
+@Directive({
+  // TODO(devversion): this selector can be removed when we update to Angular 9.0.
+  selector: 'do-not-use-abstract-mat-menu-base'
+})
 // tslint:disable-next-line:class-name
 export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnInit,
   OnDestroy {
@@ -445,6 +450,10 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
 }
 
 /** @docs-private We show the "_MatMenu" class as "MatMenu" in the docs. */
+@Directive({
+  // TODO(devversion): this selector can be removed when we update to Angular 9.0.
+  selector: 'do-not-use-abstract-mat-menu'
+})
 export class MatMenu extends _MatMenuBase {}
 
 // Note on the weird inheritance setup: we need three classes, because the MDC-based menu has to

tools/tslint-rules/noUndecoratedClassWithNgFieldsRule.ts

@@ -0,0 +1,65 @@
+import * as Lint from 'tslint';
+import * as ts from 'typescript';
+
+const RULE_FAILURE = `Undecorated class defines fields with Angular decorators. Undecorated ` +
+  `classes with Angular fields cannot be extended in Ivy since no definition is generated. ` +
+  `Add a "@Directive" decorator to fix this.`;
+
+/**
+ * Rule that doesn't allow undecorated class declarations with fields using Angular
+ * decorators.
+ */
+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.getTypeChecker()));
+  }
+}
+
+class Walker extends Lint.RuleWalker {
+  constructor(
+      sourceFile: ts.SourceFile, options: Lint.IOptions, private _typeChecker: ts.TypeChecker) {
+    super(sourceFile, options);
+  }
+
+  visitClassDeclaration(node: ts.ClassDeclaration) {
+    if (this._hasAngularDecorator(node)) {
+      return;
+    }
+
+    for (let member of node.members) {
+      if (member.decorators && this._hasAngularDecorator(member)) {
+        this.addFailureAtNode(node, RULE_FAILURE);
+        return;
+      }
+    }
+  }
+
+  /** Checks if the specified node has an Angular decorator. */
+  private _hasAngularDecorator(node: ts.Node): boolean {
+    return !!node.decorators && node.decorators.some(d => {
+      if (!ts.isCallExpression(d.expression) ||
+          !ts.isIdentifier(d.expression.expression)) {
+        return false;
+      }
+
+      const moduleImport = this._getModuleImportOfIdentifier(d.expression.expression);
+      return moduleImport ? moduleImport.startsWith('@angular/core') : false;
+    });
+  }
+
+  /** Gets the module import of the given identifier if imported. */
+  private _getModuleImportOfIdentifier(node: ts.Identifier): string|null {
+    const symbol = this._typeChecker.getSymbolAtLocation(node);
+    if (!symbol || !symbol.declarations.length) {
+      return null;
+    }
+    const decl = symbol.declarations[0];
+    if (!ts.isImportSpecifier(decl)) {
+      return null;
+    }
+    const importDecl = decl.parent.parent.parent;
+    const moduleSpecifier = importDecl.moduleSpecifier;
+    return ts.isStringLiteral(moduleSpecifier) ? moduleSpecifier.text : null;
+  }
+}

tslint.json

@@ -101,6 +101,7 @@
     "no-import-spacing": true,
     "no-private-getters": true,
     "no-undecorated-base-class-di": true,
+    "no-undecorated-class-with-ng-fields": true,
     "setters-after-getters": true,
     "ng-on-changes-property-access": true,
     "rxjs-imports": true,