Skip to content

Commit 098201d

Browse files
committed
fix(lint): enforce that module-private members have @internal.
This is needed to prevent leaking internal APIs to users via our published .d.ts typings. Fixes angular#4645 Closes angular#4989
1 parent 44188b9 commit 098201d

File tree

10 files changed

+55
-1
lines changed

10 files changed

+55
-1
lines changed

gulpfile.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,7 @@ gulp.task('lint', ['build.tools'], function() {
294294
// https://github.com/palantir/tslint#supported-rules
295295
var tslintConfig = {
296296
"rules": {
297+
"requireInternalWithUnderscore": true,
297298
"requireParameterType": true,
298299
"requireReturnType": true,
299300
"semicolon": true,

modules/angular2/src/core/change_detection/proto_change_detector.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ class _ConvertAstIntoProtoRecords implements AstVisitor {
265265
return this._addRecord(RecordType.Chain, "chain", null, args, null, 0);
266266
}
267267

268+
/** @internal */
268269
_visitAll(asts: any[]) {
269270
var res = ListWrapper.createFixedSize(asts.length);
270271
for (var i = 0; i < asts.length; ++i) {
@@ -273,6 +274,7 @@ class _ConvertAstIntoProtoRecords implements AstVisitor {
273274
return res;
274275
}
275276

277+
/** @internal */
276278
_addRecord(type, name, funcOrValue, args, fixedArgs, context) {
277279
var selfIndex = this._records.length + 1;
278280
if (context instanceof DirectiveIndex) {

modules/angular2/src/core/life_cycle/life_cycle.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ export abstract class LifeCycle {
5252

5353
@Injectable()
5454
export class LifeCycle_ extends LifeCycle {
55+
/** @internal */
5556
static _tickScope: WtfScopeFn = wtfCreateScope('LifeCycle#tick()');
56-
5757
/** @internal */
5858
_changeDetectors: ChangeDetector[];
5959
/** @internal */

modules/angular2/src/core/linker/element_injector.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,13 @@ export class DirectiveDependency extends Dependency {
119119
DirectiveDependency._attributeName(d.properties), DirectiveDependency._query(d.properties));
120120
}
121121

122+
/** @internal */
122123
static _attributeName(properties): string {
123124
var p = <AttributeMetadata>ListWrapper.find(properties, (p) => p instanceof AttributeMetadata);
124125
return isPresent(p) ? p.attributeName : null;
125126
}
126127

128+
/** @internal */
127129
static _query(properties): QueryMetadata {
128130
return <QueryMetadata>ListWrapper.find(properties, (p) => p instanceof QueryMetadata);
129131
}

modules/angular2/src/core/pipes/date_pipe.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ var defaultLocale: string = 'en-US';
8181
@Pipe({name: 'date'})
8282
@Injectable()
8383
export class DatePipe implements PipeTransform {
84+
/** @internal */
8485
static _ALIASES: {[key: string]: String} = {
8586
'medium': 'yMMMdjms',
8687
'short': 'yMdjm',

modules/angular2/src/core/pipes/number_pipe.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ var _re = RegExpWrapper.create('^(\\d+)?\\.((\\d+)(\\-(\\d+))?)?$');
2323
@CONST()
2424
@Injectable()
2525
export class NumberPipe {
26+
/** @internal */
2627
static _format(value: number, style: NumberFormatStyle, digits: string, currency: string = null,
2728
currencyAsSymbol: boolean = false): string {
2829
if (isBlank(value)) return null;

modules/angular2/src/core/render/dom/events/key_events.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ export class KeyEventsPlugin extends EventManagerPlugin {
9999
};
100100
}
101101

102+
/** @internal */
102103
static _normalizeKey(keyName: string): string {
103104
// TODO: switch to a StringMap if the mapping grows too much
104105
switch (keyName) {

modules/angular2/src/core/testability/browser_testability.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
import {global} from 'angular2/src/core/facade/lang';
88

99
class PublicTestability {
10+
/** @internal */
1011
_testability: Testability;
1112

1213
constructor(testability: Testability) { this._testability = testability; }

modules/angular2/src/web_workers/shared/client_message_broker.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ class MessageData {
142142

143143
/**
144144
* Returns the value from the StringMap if present. Otherwise returns null
145+
* @internal
145146
*/
146147
_getValueIfPresent(data: {[key: string]: any}, key: string) {
147148
if (StringMapWrapper.contains(data, key)) {
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import {RuleFailure} from 'tslint/lib/lint';
2+
import {AbstractRule} from 'tslint/lib/rules';
3+
import {RuleWalker} from 'tslint/lib/language/walker';
4+
import * as ts from 'tslint/node_modules/typescript';
5+
6+
export class Rule extends AbstractRule {
7+
public apply(sourceFile: ts.SourceFile): RuleFailure[] {
8+
const typedefWalker = new TypedefWalker(sourceFile, this.getOptions());
9+
return this.applyWithWalker(typedefWalker);
10+
}
11+
}
12+
13+
class TypedefWalker extends RuleWalker {
14+
protected visitPropertyDeclaration(node: ts.PropertyDeclaration): void {
15+
this.assertInternalAnnotationPresent(node);
16+
super.visitPropertyDeclaration(node);
17+
}
18+
19+
public visitMethodDeclaration(node: ts.MethodDeclaration): void {
20+
this.assertInternalAnnotationPresent(node);
21+
super.visitMethodDeclaration(node);
22+
}
23+
24+
private hasInternalAnnotation(range: ts.CommentRange): boolean {
25+
let text = this.getSourceFile().text;
26+
let comment = text.substring(range.pos, range.end);
27+
return comment.indexOf("@internal") >= 0;
28+
}
29+
30+
private assertInternalAnnotationPresent(node: ts.Declaration) {
31+
if (node.name.getText().charAt(0) !== '_') return;
32+
if (node.modifiers && node.modifiers.flags & ts.NodeFlags.Private) return;
33+
34+
const ranges = ts.getLeadingCommentRanges(this.getSourceFile().text, node.pos);
35+
if (ranges) {
36+
for (let i = 0; i < ranges.length; i++) {
37+
if (this.hasInternalAnnotation(ranges[i])) return;
38+
}
39+
}
40+
this.addFailure(this.createFailure(
41+
node.getStart(), node.getWidth(),
42+
`module-private member ${node.name.getText()} must be annotated @internal`));
43+
}
44+
}

0 commit comments

Comments
 (0)