From 28ede304fb82d8589ccb375fd656a9dd7ced7008 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 1 Sep 2017 17:05:46 +0200 Subject: [PATCH] build: stricter unused variable checking (#6633) Currently only variables that don't start with an underscore will be checked by TSLint. This doesn't make sense, because variables get prefixed with an underscore to indicate that they are not part of the public API, but still need to be public in favor of AOT compilation for example. This means that those properties are having the public modifier set (explicitly or automatically) and therefore won't be treated as unused anyway. **TL;DR**: With this change, prefixed variables will be checked by the `no-unused-variable` TSLint rule and those which are marked as `private` and are unused will be catched by TSLint. --- src/cdk/overlay/position/connected-position-strategy.ts | 6 +++--- src/cdk/overlay/position/overlay-position-builder.ts | 2 +- src/cdk/rxjs/rx-operators.ts | 3 ++- src/lib/dialog/dialog-container.ts | 2 -- src/lib/dialog/dialog.ts | 6 +++--- src/lib/expansion/expansion-panel-header.ts | 4 ++-- src/lib/paginator/paginator.spec.ts | 4 +--- src/lib/select/select.ts | 1 - src/lib/slide-toggle/slide-toggle.ts | 6 +++--- src/lib/tabs/tab-header.spec.ts | 4 ++-- src/lib/tooltip/tooltip.ts | 6 +++--- tools/screenshot-test/src/app/viewer/viewer.component.ts | 6 +++--- tslint.json | 2 +- 13 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/cdk/overlay/position/connected-position-strategy.ts b/src/cdk/overlay/position/connected-position-strategy.ts index 101e8c23bdf..19dc0c76159 100644 --- a/src/cdk/overlay/position/connected-position-strategy.ts +++ b/src/cdk/overlay/position/connected-position-strategy.ts @@ -72,12 +72,12 @@ export class ConnectedPositionStrategy implements PositionStrategy { } constructor( + originPos: OriginConnectionPosition, + overlayPos: OverlayConnectionPosition, private _connectedTo: ElementRef, - private _originPos: OriginConnectionPosition, - private _overlayPos: OverlayConnectionPosition, private _viewportRuler: ViewportRuler) { this._origin = this._connectedTo.nativeElement; - this.withFallbackPosition(_originPos, _overlayPos); + this.withFallbackPosition(originPos, overlayPos); } /** Ordered list of preferred positions, from most to least desirable. */ diff --git a/src/cdk/overlay/position/overlay-position-builder.ts b/src/cdk/overlay/position/overlay-position-builder.ts index 1ce90297093..58e2d44dc2e 100644 --- a/src/cdk/overlay/position/overlay-position-builder.ts +++ b/src/cdk/overlay/position/overlay-position-builder.ts @@ -36,6 +36,6 @@ export class OverlayPositionBuilder { elementRef: ElementRef, originPos: OriginConnectionPosition, overlayPos: OverlayConnectionPosition): ConnectedPositionStrategy { - return new ConnectedPositionStrategy(elementRef, originPos, overlayPos, this._viewportRuler); + return new ConnectedPositionStrategy(originPos, overlayPos, elementRef, this._viewportRuler); } } diff --git a/src/cdk/rxjs/rx-operators.ts b/src/cdk/rxjs/rx-operators.ts index a404ac2a47a..ba0c07f96e0 100644 --- a/src/cdk/rxjs/rx-operators.ts +++ b/src/cdk/rxjs/rx-operators.ts @@ -76,7 +76,7 @@ export interface StrictRxChain { result(): Observable; } - +/* tslint:disable:no-unused-variable */ export class FinallyBrand { private _; } export class CatchBrand { private _; } export class DoBrand { private _; } @@ -89,6 +89,7 @@ export class StartWithBrand { private _; } export class DebounceTimeBrand { private _; } export class AuditTimeBrand { private _; } export class TakeUntilBrand { private _; } +/* tslint:enable:no-unused-variable */ export type finallyOperatorType = typeof _finallyOperator & FinallyBrand; diff --git a/src/lib/dialog/dialog-container.ts b/src/lib/dialog/dialog-container.ts index 83bc4a1d8c7..40e48f3c4e0 100644 --- a/src/lib/dialog/dialog-container.ts +++ b/src/lib/dialog/dialog-container.ts @@ -13,7 +13,6 @@ import { EmbeddedViewRef, EventEmitter, Inject, - NgZone, Optional, ChangeDetectorRef, ViewChild, @@ -100,7 +99,6 @@ export class MdDialogContainer extends BasePortalHost { _isAnimating = false; constructor( - private _ngZone: NgZone, private _elementRef: ElementRef, private _focusTrapFactory: FocusTrapFactory, private _changeDetectorRef: ChangeDetectorRef, diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index 1d920f0072f..784459779bb 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -96,15 +96,15 @@ export class MdDialog { constructor( private _overlay: Overlay, private _injector: Injector, + @Optional() location: Location, @Inject(MD_DIALOG_SCROLL_STRATEGY) private _scrollStrategy, - @Optional() private _location: Location, @Optional() @SkipSelf() private _parentDialog: MdDialog) { // Close all of the dialogs when the user goes forwards/backwards in history or when the // location hash changes. Note that this usually doesn't include clicking on links (unless // the user is using the `HashLocationStrategy`). - if (!_parentDialog && _location) { - _location.subscribe(() => this.closeAll()); + if (!_parentDialog && location) { + location.subscribe(() => this.closeAll()); } } diff --git a/src/lib/expansion/expansion-panel-header.ts b/src/lib/expansion/expansion-panel-header.ts index 818c30a45f8..05cc1a2d71b 100644 --- a/src/lib/expansion/expansion-panel-header.ts +++ b/src/lib/expansion/expansion-panel-header.ts @@ -90,8 +90,8 @@ export class MdExpansionPanelHeader implements OnDestroy { private _parentChangeSubscription = Subscription.EMPTY; constructor( + renderer: Renderer2, @Host() public panel: MdExpansionPanel, - private _renderer: Renderer2, private _element: ElementRef, private _focusOriginMonitor: FocusOriginMonitor, private _changeDetectorRef: ChangeDetectorRef) { @@ -105,7 +105,7 @@ export class MdExpansionPanelHeader implements OnDestroy { ) .subscribe(() => this._changeDetectorRef.markForCheck()); - _focusOriginMonitor.monitor(_element.nativeElement, _renderer, false); + _focusOriginMonitor.monitor(_element.nativeElement, renderer, false); } /** Height of the header while the panel is expanded. */ diff --git a/src/lib/paginator/paginator.spec.ts b/src/lib/paginator/paginator.spec.ts index 7686df86a0c..bf3d9cd96ef 100644 --- a/src/lib/paginator/paginator.spec.ts +++ b/src/lib/paginator/paginator.spec.ts @@ -1,7 +1,7 @@ import {async, ComponentFixture, TestBed, inject} from '@angular/core/testing'; import {MdPaginatorModule} from './index'; import {MdPaginator, PageEvent} from './paginator'; -import {Component, ElementRef, ViewChild} from '@angular/core'; +import {Component, ViewChild} from '@angular/core'; import {MdPaginatorIntl} from './paginator-intl'; import {NoopAnimationsModule} from '@angular/platform-browser/animations'; import {dispatchMouseEvent} from '@angular/cdk/testing'; @@ -281,8 +281,6 @@ class MdPaginatorApp { @ViewChild(MdPaginator) mdPaginator: MdPaginator; - constructor(private _elementRef: ElementRef) { } - goToLastPage() { this.pageIndex = Math.ceil(this.length / this.pageSize); } diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 97730a267f5..26b825faf49 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -428,7 +428,6 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On constructor( private _viewportRuler: ViewportRuler, private _changeDetectorRef: ChangeDetectorRef, - private _overlay: Overlay, private _platform: Platform, renderer: Renderer2, elementRef: ElementRef, diff --git a/src/lib/slide-toggle/slide-toggle.ts b/src/lib/slide-toggle/slide-toggle.ts index 07b4e69bf61..28c4eaf0742 100644 --- a/src/lib/slide-toggle/slide-toggle.ts +++ b/src/lib/slide-toggle/slide-toggle.ts @@ -301,12 +301,12 @@ class SlideToggleRenderer { /** Whether the thumb is currently being dragged. */ dragging: boolean = false; - constructor(private _elementRef: ElementRef, platform: Platform) { + constructor(elementRef: ElementRef, platform: Platform) { // We only need to interact with these elements when we're on the browser, so only grab // the reference in that case. if (platform.isBrowser) { - this._thumbEl = _elementRef.nativeElement.querySelector('.mat-slide-toggle-thumb-container'); - this._thumbBarEl = _elementRef.nativeElement.querySelector('.mat-slide-toggle-bar'); + this._thumbEl = elementRef.nativeElement.querySelector('.mat-slide-toggle-thumb-container'); + this._thumbBarEl = elementRef.nativeElement.querySelector('.mat-slide-toggle-bar'); } } diff --git a/src/lib/tabs/tab-header.spec.ts b/src/lib/tabs/tab-header.spec.ts index 7d9ff944307..64239dfc60a 100644 --- a/src/lib/tabs/tab-header.spec.ts +++ b/src/lib/tabs/tab-header.spec.ts @@ -1,7 +1,7 @@ import { async, ComponentFixture, TestBed, fakeAsync, tick, discardPeriodicTasks } from '@angular/core/testing'; -import {Component, ViewChild, ViewContainerRef} from '@angular/core'; +import {Component, ViewChild} from '@angular/core'; import {CommonModule} from '@angular/common'; import {By} from '@angular/platform-browser'; import {ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE} from '@angular/cdk/keycodes'; @@ -328,7 +328,7 @@ class SimpleTabHeaderApp { @ViewChild(MdTabHeader) mdTabHeader: MdTabHeader; - constructor(private _viewContainerRef: ViewContainerRef) { + constructor() { this.tabs[this.disabledTabIndex].disabled = true; } diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 1a4f1d699af..0defc0add49 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -197,12 +197,12 @@ export class MdTooltip implements OnDestroy { private _leaveListener: Function; constructor( + renderer: Renderer2, private _overlay: Overlay, private _elementRef: ElementRef, private _scrollDispatcher: ScrollDispatcher, private _viewContainerRef: ViewContainerRef, private _ngZone: NgZone, - private _renderer: Renderer2, private _platform: Platform, @Inject(MD_TOOLTIP_SCROLL_STRATEGY) private _scrollStrategy, @Optional() private _dir: Directionality) { @@ -211,9 +211,9 @@ export class MdTooltip implements OnDestroy { // they can prevent the first tap from firing its click event. if (!_platform.IOS) { this._enterListener = - _renderer.listen(_elementRef.nativeElement, 'mouseenter', () => this.show()); + renderer.listen(_elementRef.nativeElement, 'mouseenter', () => this.show()); this._leaveListener = - _renderer.listen(_elementRef.nativeElement, 'mouseleave', () => this.hide()); + renderer.listen(_elementRef.nativeElement, 'mouseleave', () => this.hide()); } } diff --git a/tools/screenshot-test/src/app/viewer/viewer.component.ts b/tools/screenshot-test/src/app/viewer/viewer.component.ts index 71ce327bd18..68f159aceac 100644 --- a/tools/screenshot-test/src/app/viewer/viewer.component.ts +++ b/tools/screenshot-test/src/app/viewer/viewer.component.ts @@ -38,9 +38,9 @@ export class ViewerComponent { } constructor(private _service: FirebaseService, - private _route: ActivatedRoute, - public snackBar: MdSnackBar) { - _route.params.subscribe(p => { + public snackBar: MdSnackBar, + activatedRoute: ActivatedRoute) { + activatedRoute.params.subscribe(p => { this._service.prNumber = p['id']; }); } diff --git a/tslint.json b/tslint.json index a1907e04ecb..911ffe3cf7a 100644 --- a/tslint.json +++ b/tslint.json @@ -29,7 +29,7 @@ "no-var-keyword": true, "member-access": [true, "no-public"], "no-debugger": true, - "no-unused-variable": [true, {"ignore-pattern": "^_"}], + "no-unused-variable": true, "one-line": [ true, "check-catch",