Skip to content

Commit 4b2af08

Browse files
committed
Verify type annotaions on public APIs (#24).
* Implements #24. * Tweaks the rule boilerplate generator. BUG= [email protected] Review URL: https://codereview.chromium.org//1407283008 .
1 parent 154dbfe commit 4b2af08

File tree

5 files changed

+208
-9
lines changed

5 files changed

+208
-9
lines changed

lib/src/ast.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import 'package:analyzer/src/generated/ast.dart'
2424
FieldDeclaration,
2525
FunctionDeclaration,
2626
FunctionTypeAlias,
27+
Identifier,
2728
MethodDeclaration,
2829
ReturnStatement,
2930
SimpleIdentifier,
@@ -243,6 +244,10 @@ AstNode _getNodeToAnnotate(Declaration node) {
243244
return null;
244245
}
245246

247+
/// Check if the given identifier has a private name.
248+
bool isPrivate(SimpleIdentifier identifier) =>
249+
identifier != null ? Identifier.isPrivateName(identifier.name) : false;
250+
246251
/// An [Element] processor function type.
247252
/// If `true` is returned, children of [element] will be visited.
248253
typedef bool ElementProcessor(Element element);

lib/src/rules.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import 'package:linter/src/rules/prefer_is_not_empty.dart';
2424
import 'package:linter/src/rules/pub/package_names.dart';
2525
import 'package:linter/src/rules/slash_for_doc_comments.dart';
2626
import 'package:linter/src/rules/super_goes_last.dart';
27+
import 'package:linter/src/rules/type_annotate_public_apis.dart';
2728
import 'package:linter/src/rules/type_init_formals.dart';
2829
import 'package:linter/src/rules/unnecessary_brace_in_string_interp.dart';
2930
import 'package:linter/src/rules/unnecessary_getters_setters.dart';
@@ -46,6 +47,7 @@ final Registry ruleRegistry = new Registry()
4647
..register(new SlashForDocComments())
4748
..register(new SuperGoesLast())
4849
..register(new TypeInitFormals())
50+
..register(new TypeAnnotatePublicApis())
4951
..register(new UnnecessaryBraceInStringInterp())
5052
// Disabled pending fix: https://github.com/dart-lang/linter/issues/35
5153
//..register(new UnnecessaryGetters())
@@ -68,7 +70,7 @@ class Registry extends Object with IterableMixin<LintRule> {
6870
/// my_rule: true
6971
///
7072
/// enables `my_rule`.
71-
///
73+
///
7274
/// Unspecified rules are treated as disabled by default.
7375
Iterable<LintRule> enabled(LintConfig config) => rules
7476
.where((rule) => config.ruleConfigs.any((rc) => rc.enables(rule.name)));
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
library linter.src.rules.type_annotate_public_apis;
6+
7+
import 'package:analyzer/src/generated/ast.dart';
8+
import 'package:linter/src/linter.dart';
9+
import 'package:linter/src/ast.dart';
10+
11+
const desc = r'Type annotate public APIs.';
12+
13+
const details = r'''
14+
From [effective dart] (https://www.dartlang.org/effective-dart/design/#do-type-annotate-public-apis):
15+
16+
**DO** type annotate public APIs.
17+
18+
Type annotations are important documentation for how a library should be used.
19+
Annotating the parameter and return types of public methods and functions helps
20+
users understand what the API expects and what it provides.
21+
22+
Note that if a public API accepts a range of values that Dart's type system
23+
cannot express, then it is acceptable to leave that untyped. In that case, the
24+
implicit `dynamic` is the correct type for the API.
25+
26+
For code internal to a library (either private, or things like nested functions)
27+
annotate where you feel it helps, but don't feel that you *must* provide them.
28+
29+
**BAD:**
30+
```
31+
install(id, destination) {
32+
// ...
33+
}
34+
```
35+
36+
Here, it's unclear what `id` is. A string? And what is `destination`? A string
37+
or a `File` object? Is this method synchronous or asynchronous?
38+
39+
40+
**GOOD:**
41+
42+
```
43+
Future<bool> install(PackageId id, String destination) {
44+
// ...
45+
}
46+
```
47+
48+
With types, all of this is clarified.
49+
''';
50+
51+
class TypeAnnotatePublicApis extends LintRule {
52+
TypeAnnotatePublicApis()
53+
: super(
54+
name: 'type_annotate_public_apis',
55+
description: desc,
56+
details: details,
57+
group: Group.style);
58+
59+
@override
60+
AstVisitor getVisitor() => new Visitor(this);
61+
}
62+
63+
class Visitor extends SimpleAstVisitor {
64+
final LintRule rule;
65+
_VisitoHelper v;
66+
Visitor(this.rule) {
67+
v = new _VisitoHelper(rule);
68+
}
69+
70+
@override
71+
visitFieldDeclaration(FieldDeclaration node) {
72+
if (node.fields.type == null) {
73+
node.fields.accept(v);
74+
}
75+
}
76+
77+
@override
78+
visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
79+
if (node.variables.type == null) {
80+
node.variables.accept(v);
81+
}
82+
}
83+
84+
@override
85+
visitFunctionDeclaration(FunctionDeclaration node) {
86+
if (!isPrivate(node.name)) {
87+
if (node.returnType == null) {
88+
rule.reportLint(node.name);
89+
} else {
90+
node.functionExpression.parameters.accept(v);
91+
}
92+
}
93+
}
94+
95+
@override
96+
visitMethodDeclaration(MethodDeclaration node) {
97+
if (!isPrivate(node.name)) {
98+
if (node.returnType == null) {
99+
rule.reportLint(node.name);
100+
} else {
101+
node.parameters.accept(v);
102+
}
103+
}
104+
}
105+
106+
@override
107+
visitFunctionTypeAlias(FunctionTypeAlias node) {
108+
if (node.returnType == null) {
109+
rule.reportLint(node.name);
110+
} else {
111+
node.parameters.accept(v);
112+
}
113+
}
114+
}
115+
116+
class _VisitoHelper extends RecursiveAstVisitor {
117+
final LintRule rule;
118+
_VisitoHelper(this.rule);
119+
120+
@override
121+
visitVariableDeclaration(VariableDeclaration node) {
122+
if (!isPrivate(node.name)) {
123+
rule.reportLint(node.name);
124+
}
125+
}
126+
127+
@override
128+
visitSimpleFormalParameter(SimpleFormalParameter param) {
129+
if (param.type == null) {
130+
rule.reportLint(param);
131+
}
132+
}
133+
}
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
const X = ''; //LINT
6+
7+
f() {} //LINT
8+
9+
void g(x) {} //LINT
10+
11+
typedef Foo(x); //LINT
12+
13+
typedef void Bar(int x);
14+
15+
_f() {}
16+
const _X = '';
17+
18+
class A {
19+
20+
var x; // LINT
21+
final xx = 1; //LINT
22+
static const y = ''; //LINT
23+
static final z = 3; //LINT
24+
25+
var zzz, //LINT
26+
_zzz;
27+
28+
f() {} //LINT
29+
void g(x) {} //LINT
30+
static h() {} //LINT
31+
static void j(x) {} //LINT
32+
static void k(var v) {} //LINT
33+
34+
var _x;
35+
final _xx = 1;
36+
static const _y = '';
37+
static final _z = 3;
38+
39+
void m() {}
40+
41+
_f() {}
42+
void _g(x) {}
43+
static _h() {}
44+
static _j(x) {}
45+
static _k(var x) {}
46+
}

tool/rule.dart

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ ${parser.usage}
8383
String toClassName(String libName) =>
8484
libName.split('_').map((bit) => capitalize(bit)).join();
8585

86-
String _generateStub(String libName, String className) => '''
86+
String _generateStub(String libName, String className) => """
8787
// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file
8888
// for details. All rights reserved. Use of this source code is governed by a
8989
// BSD-style license that can be found in the LICENSE file.
@@ -96,27 +96,40 @@ import 'package:linter/src/linter.dart';
9696
9797
const desc = r' ';
9898
99-
const details = r' ';
99+
const details = r'''
100+
101+
**DO** ...
102+
103+
**BAD:**
104+
```
105+
106+
```
107+
108+
**GOOD:**
109+
```
110+
111+
```
112+
113+
''';
100114
101115
class $className extends LintRule {
102116
$className() : super(
103117
name: '$libName',
104-
description: desc,
105-
details: details,
106-
group: Group.STYLE_GUIDE,
107-
kind: Kind.AVOID);
118+
description: desc,
119+
details: details,
120+
group: Group.style);
108121
109122
@override
110123
AstVisitor getVisitor() => new Visitor(this);
111124
}
112125
113126
class Visitor extends SimpleAstVisitor {
114-
LintRule rule;
127+
final LintRule rule;
115128
Visitor(this.rule);
116129
117130
118131
}
119-
''';
132+
""";
120133

121134
String _generateTest(String libName, String className) => '''
122135
// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file

0 commit comments

Comments
 (0)