build: add tslint rule to disallow private setters (#12706)

Adds a tslint rule to prevent people from adding private getters since they don't contribute to the public API and they generate more ES5 code.

Author: crisbeto

src/cdk/stepper/stepper.ts

@@ -117,14 +117,14 @@ export class CdkStep implements OnChanges {
   /** Whether step is marked as completed. */
   @Input()
   get completed(): boolean {
-    return this._customCompleted == null ? this._defaultCompleted : this._customCompleted;
+    return this._customCompleted == null ? this._defaultCompleted() : this._customCompleted;
   }
   set completed(value: boolean) {
     this._customCompleted = coerceBooleanProperty(value);
   }
   private _customCompleted: boolean | null = null;
 
-  private get _defaultCompleted() {
+  private _defaultCompleted() {
     return this.stepControl ? this.stepControl.valid && this.interacted : this.interacted;
   }
 

src/lib/autocomplete/autocomplete-trigger.ts

@@ -250,7 +250,7 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
       this.optionSelections,
       this.autocomplete._keyManager.tabOut.pipe(filter(() => this._overlayAttached)),
       this._closeKeyEventStream,
-      this._outsideClickStream,
+      this._getOutsideClickStream(),
       this._overlayRef ?
           this._overlayRef.detachments().pipe(filter(() => this._overlayAttached)) :
           observableOf()
@@ -283,7 +283,7 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
   }
 
   /** Stream of clicks outside of the autocomplete panel. */
-  private get _outsideClickStream(): Observable<any> {
+  private _getOutsideClickStream(): Observable<any> {
     if (!this._document) {
       return observableOf(null);
     }

src/lib/menu/menu.spec.ts

@@ -777,7 +777,7 @@ describe('MatMenu', () => {
       }
 
       get overlayRect() {
-        return this.overlayPane.getBoundingClientRect();
+        return this.getOverlayPane().getBoundingClientRect();
       }
 
       get triggerRect() {
@@ -788,7 +788,7 @@ describe('MatMenu', () => {
         return overlayContainerElement.querySelector('.mat-menu-panel');
       }
 
-      private get overlayPane() {
+      private getOverlayPane() {
         return overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement;
       }
     }

src/lib/slider/slider.ts

@@ -348,7 +348,7 @@ export class MatSlider extends _MatSliderMixinBase
   get _trackBackgroundStyles(): { [key: string]: string } {
     const axis = this.vertical ? 'Y' : 'X';
     const scale = this.vertical ? `1, ${1 - this.percent}, 1` : `${1 - this.percent}, 1, 1`;
-    const sign = this._invertMouseCoords ? '-' : '';
+    const sign = this._shouldInvertMouseCoords() ? '-' : '';
 
     return {
       // scale3d avoids some rendering issues in Chrome. See #12071.
@@ -360,7 +360,7 @@ export class MatSlider extends _MatSliderMixinBase
   get _trackFillStyles(): { [key: string]: string } {
     const axis = this.vertical ? 'Y' : 'X';
     const scale = this.vertical ? `1, ${this.percent}, 1` : `${this.percent}, 1, 1`;
-    const sign = this._invertMouseCoords ? '' : '-';
+    const sign = this._shouldInvertMouseCoords() ? '' : '-';
 
     return {
       // scale3d avoids some rendering issues in Chrome. See #12071.
@@ -373,7 +373,7 @@ export class MatSlider extends _MatSliderMixinBase
     let axis = this.vertical ? 'Y' : 'X';
     // For a horizontal slider in RTL languages we push the ticks container off the left edge
     // instead of the right edge to avoid causing a horizontal scrollbar to appear.
-    let sign = !this.vertical && this._direction == 'rtl' ? '' : '-';
+    let sign = !this.vertical && this._getDirection() == 'rtl' ? '' : '-';
     let offset = this._tickIntervalPercent / 2 * 100;
     return {
       'transform': `translate${axis}(${sign}${offset}%)`
@@ -388,8 +388,8 @@ export class MatSlider extends _MatSliderMixinBase
     // Depending on the direction we pushed the ticks container, push the ticks the opposite
     // direction to re-center them but clip off the end edge. In RTL languages we need to flip the
     // ticks 180 degrees so we're really cutting off the end edge abd not the start.
-    let sign = !this.vertical && this._direction == 'rtl' ? '-' : '';
-    let rotate = !this.vertical && this._direction == 'rtl' ? ' rotate(180deg)' : '';
+    let sign = !this.vertical && this._getDirection() == 'rtl' ? '-' : '';
+    let rotate = !this.vertical && this._getDirection() == 'rtl' ? ' rotate(180deg)' : '';
     let styles: { [key: string]: string } = {
       'backgroundSize': backgroundSize,
       // Without translateZ ticks sometimes jitter as the slider moves on Chrome & Firefox.
@@ -411,7 +411,7 @@ export class MatSlider extends _MatSliderMixinBase
     // For a horizontal slider in RTL languages we push the thumb container off the left edge
     // instead of the right edge to avoid causing a horizontal scrollbar to appear.
     let invertOffset =
-        (this._direction == 'rtl' && !this.vertical) ? !this._invertAxis : this._invertAxis;
+        (this._getDirection() == 'rtl' && !this.vertical) ? !this._invertAxis : this._invertAxis;
     let offset = (invertOffset ? this.percent : 1 - this.percent) * 100;
     return {
       'transform': `translate${axis}(-${offset}%)`
@@ -442,12 +442,12 @@ export class MatSlider extends _MatSliderMixinBase
    * Whether mouse events should be converted to a slider position by calculating their distance
    * from the right or bottom edge of the slider as opposed to the top or left.
    */
-  private get _invertMouseCoords() {
-    return (this._direction == 'rtl' && !this.vertical) ? !this._invertAxis : this._invertAxis;
+  private _shouldInvertMouseCoords() {
+    return (this._getDirection() == 'rtl' && !this.vertical) ? !this._invertAxis : this._invertAxis;
   }
 
   /** The language direction for this slider element. */
-  private get _direction() {
+  private _getDirection() {
     return (this._dir && this._dir.value == 'rtl') ? 'rtl' : 'ltr';
   }
 
@@ -597,14 +597,14 @@ export class MatSlider extends _MatSliderMixinBase
         // expect left to mean increment. Therefore we flip the meaning of the side arrow keys for
         // RTL. For inverted sliders we prefer a good a11y experience to having it "look right" for
         // sighted users, therefore we do not swap the meaning.
-        this._increment(this._direction == 'rtl' ? 1 : -1);
+        this._increment(this._getDirection() == 'rtl' ? 1 : -1);
         break;
       case UP_ARROW:
         this._increment(1);
         break;
       case RIGHT_ARROW:
         // See comment on LEFT_ARROW about the conditions under which we flip the meaning.
-        this._increment(this._direction == 'rtl' ? -1 : 1);
+        this._increment(this._getDirection() == 'rtl' ? -1 : 1);
         break;
       case DOWN_ARROW:
         this._increment(-1);
@@ -646,7 +646,7 @@ export class MatSlider extends _MatSliderMixinBase
     // The exact value is calculated from the event and used to find the closest snap value.
     let percent = this._clamp((posComponent - offset) / size);
 
-    if (this._invertMouseCoords) {
+    if (this._shouldInvertMouseCoords()) {
       percent = 1 - percent;
     }
 

tools/package-tools/build-package.ts

@@ -39,7 +39,7 @@ export class BuildPackage {
   private bundler = new PackageBundler(this);
 
   /** Secondary entry-points partitioned by their build depth. */
-  private get secondaryEntryPointsByDepth(): string[][] {
+  get secondaryEntryPointsByDepth(): string[][] {
     this.cacheSecondaryEntryPoints();
     return this._secondaryEntryPointsByDepth;
   }

tools/tslint-rules/noPrivateGettersRule.ts

@@ -0,0 +1,39 @@
+import * as ts from 'typescript';
+import * as Lint from 'tslint';
+import * as tsutils from 'tsutils';
+
+/**
+ * Rule that doesn't allow private getters.
+ */
+export class Rule extends Lint.Rules.AbstractRule {
+  apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+    return this.applyWithWalker(new Walker(sourceFile, this.getOptions()));
+  }
+}
+
+class Walker extends Lint.RuleWalker {
+  visitGetAccessor(getter: ts.GetAccessorDeclaration) {
+    // Check whether the getter is private.
+    if (!getter.modifiers || !getter.modifiers.find(m => m.kind === ts.SyntaxKind.PrivateKeyword)) {
+      return super.visitGetAccessor(getter);
+    }
+
+    // Check that it's inside a class.
+    if (!getter.parent || !tsutils.isClassDeclaration(getter.parent)) {
+      return super.visitGetAccessor(getter);
+    }
+
+    const getterName = getter.name.getText();
+    const setter = getter.parent.members.find(member => {
+      return tsutils.isSetAccessorDeclaration(member) && member.name.getText() === getterName;
+    }) as ts.SetAccessorDeclaration | undefined;
+
+    // Only log a failure if it doesn't have a corresponding setter.
+    if (!setter) {
+      this.addFailureAtNode(getter, 'Private setters generate unnecessary ' +
+                                    'code. Use a function instead.');
+    }
+
+    return super.visitGetAccessor(getter);
+  }
+}

tslint.json

@@ -98,6 +98,7 @@
     "ts-loader": true,
     "no-exposed-todo": true,
     "no-import-spacing": true,
+    "no-private-getters": true,
     "setters-after-getters": true,
     "rxjs-imports": true,
     "no-host-decorator-in-concrete": [