Skip to content

build: add tslint rule to disallow private setters #12706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/cdk/stepper/stepper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/lib/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,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()
Expand Down Expand Up @@ -282,7 +282,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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ describe('MatMenu', () => {
}

get overlayRect() {
return this.overlayPane.getBoundingClientRect();
return this.getOverlayPane().getBoundingClientRect();
}

get triggerRect() {
Expand All @@ -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;
}
}
Expand Down
24 changes: 12 additions & 12 deletions src/lib/slider/slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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}%)`
Expand All @@ -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.
Expand All @@ -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}%)`
Expand Down Expand Up @@ -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';
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion tools/package-tools/build-package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
39 changes: 39 additions & 0 deletions tools/tslint-rules/noPrivateGettersRule.ts
Original file line number Diff line number Diff line change
@@ -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);
}
}
1 change: 1 addition & 0 deletions tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down