Skip to content

Commit 4d56a1e

Browse files
Tim Blasimhevery
authored andcommitted
chore: Fixing review comments on Dart transformers
See https://codereview.chromium.org/927373004/ Closes angular#705
1 parent fb5b168 commit 4d56a1e

File tree

9 files changed

+41
-42
lines changed

9 files changed

+41
-42
lines changed

modules/angular2/src/transform/annotation_processor.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
library angular2.src.transform;
2+
13
import 'dart:collection' show Queue;
24
import 'package:analyzer/src/generated/element.dart';
35

46
/// Provides a mechanism for checking an element for the provided
57
/// [_annotationClass] and reporting the resulting (element, annotation) pairs.
68
class AnnotationMatcher {
79
/// Queue for annotations.
8-
final initQueue = new Queue<AnnotationMatch>();
10+
final matchQueue = new Queue<AnnotationMatch>();
911
/// All the annotations we have seen for each element
1012
final _seenAnnotations = new Map<Element, Set<ElementAnnotation>>();
1113

@@ -16,7 +18,7 @@ class AnnotationMatcher {
1618

1719
/// Records all [_annotationClass] annotations and the [element]s they apply to.
1820
/// Returns [true] if 1) [element] is annotated with [_annotationClass] and
19-
/// 2) ([element], [_annotationClass]) has been seen previously.
21+
/// 2) ([element], [_annotationClass]) has not been seen previously.
2022
bool processAnnotations(ClassElement element) {
2123
var found = false;
2224
element.metadata.where((ElementAnnotation meta) {
@@ -31,7 +33,7 @@ class AnnotationMatcher {
3133
.contains(meta);
3234
}).forEach((ElementAnnotation meta) {
3335
_seenAnnotations[element].add(meta);
34-
initQueue.addLast(new AnnotationMatch(element, meta));
36+
matchQueue.addLast(new AnnotationMatch(element, meta));
3537
found = true;
3638
});
3739
return found;

modules/angular2/src/transform/codegen.dart

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
library angular2.transformer;
1+
library angular2.src.transform;
22

33
import 'package:analyzer/src/generated/ast.dart';
44
import 'package:analyzer/src/generated/element.dart';
@@ -17,6 +17,8 @@ class Context {
1717
final Map<LibraryElement, String> _libraryPrefixes;
1818

1919
DirectiveRegistry _directiveRegistry;
20+
/// Generates [registerType] calls for all [register]ed [AnnotationMatch]
21+
/// objects.
2022
DirectiveRegistry get directiveRegistry => _directiveRegistry;
2123

2224
Context({TransformLogger logger})
@@ -26,33 +28,31 @@ class Context {
2628
}
2729

2830
void error(String errorString) {
29-
if (_logger != null) {
30-
_logger.error(errorString);
31-
} else {
32-
throw new Error(errorString);
33-
}
31+
if (_logger == null) throw new Error(errorString);
32+
_logger.error(errorString);
3433
}
3534

3635
/// If elements in [lib] should be prefixed in our generated code, returns
3736
/// the appropriate prefix followed by a `.`. Future items from the same
3837
/// library will use the same prefix.
3938
/// If [lib] does not need a prefix, returns the empty string.
4039
String _getPrefixDot(LibraryElement lib) {
41-
var prefix = lib != null && !lib.isInSdk
42-
? _libraryPrefixes.putIfAbsent(lib, () => 'i${_libraryPrefixes.length}')
43-
: null;
44-
return prefix == null ? '' : '${prefix}.';
40+
if (lib == null || lib.isInSdk) return '';
41+
var prefix =
42+
_libraryPrefixes.putIfAbsent(lib, () => 'i${_libraryPrefixes.length}');
43+
return '${prefix}.';
4544
}
4645
}
4746

47+
/// Object which [register]s [AnnotationMatch] objects for code generation.
4848
abstract class DirectiveRegistry {
4949
// Adds [entry] to the `registerType` calls which will be generated.
5050
void register(AnnotationMatch entry);
5151
}
5252

53-
const _reflectorImport =
54-
'import \'package:angular2/src/reflection/reflection.dart\' '
55-
'show reflector;';
53+
const _reflectorImport = '''
54+
import 'package:angular2/src/reflection/reflection.dart' show reflector;
55+
''';
5656

5757
/// Default implementation to map from [LibraryElement] to [AssetId]. This
5858
/// assumes that [el.source] has a getter called [assetId].
@@ -102,6 +102,8 @@ _codegenImport(Context context, AssetId libraryId, AssetId entryPoint) {
102102
}
103103
}
104104

105+
// TODO(https://github.com/kegluneq/angular/issues/4): Remove calls to
106+
// Element#node.
105107
class _DirectiveRegistryImpl implements DirectiveRegistry {
106108
final Context _context;
107109
final StringBuffer _buffer = new StringBuffer();
@@ -197,33 +199,24 @@ abstract class _TransformVisitor extends ToSourceVisitor {
197199
: this._writer = writer,
198200
super(writer);
199201

200-
/// Safely visit the given node.
201-
/// @param node the node to be visited
202+
/// Safely visit [node].
202203
void _visitNode(AstNode node) {
203204
if (node != null) {
204205
node.accept(this);
205206
}
206207
}
207208

208-
/**
209-
* Safely visit the given node, printing the prefix before the node if it is non-`null`.
210-
*
211-
* @param prefix the prefix to be printed if there is a node to visit
212-
* @param node the node to be visited
213-
*/
209+
/// If [node] is null does nothing. Otherwise, prints [prefix], then
210+
/// visits [node].
214211
void _visitNodeWithPrefix(String prefix, AstNode node) {
215212
if (node != null) {
216213
_writer.print(prefix);
217214
node.accept(this);
218215
}
219216
}
220217

221-
/**
222-
* Safely visit the given node, printing the suffix after the node if it is non-`null`.
223-
*
224-
* @param suffix the suffix to be printed if there is a node to visit
225-
* @param node the node to be visited
226-
*/
218+
/// If [node] is null does nothing. Otherwise, visits [node], then prints
219+
/// [suffix].
227220
void _visitNodeWithSuffix(AstNode node, String suffix) {
228221
if (node != null) {
229222
node.accept(this);

modules/angular2/src/transform/html_transform.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
library angular2.transformer;
1+
library angular2.src.transform;
22

33
import 'dart:async';
44
import 'package:barback/barback.dart';

modules/angular2/src/transform/options.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
library angular2.transformer;
1+
library angular2.src.transform;
22

33
import 'package:path/path.dart' as path;
44

modules/angular2/src/transform/resolvers.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
library angular2.src.transform;
2+
13
import 'package:code_transformers/resolver.dart';
24

35
Resolvers createResolvers() {

modules/angular2/src/transform/transformer.dart

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
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-
library angular2.transformer;
1+
library angular2.src.transform;
52

63
import 'dart:async';
74
import 'package:barback/barback.dart';
@@ -60,9 +57,12 @@ class AngularTransformer extends Transformer {
6057
.error('New entry point file $newEntryPointId already exists.');
6158
} else {
6259
return _resolvers.get(transform).then((resolver) {
63-
new _BootstrapFileBuilder(resolver, transform,
64-
transform.primaryInput.id, newEntryPointId).run();
65-
resolver.release();
60+
try {
61+
new _BootstrapFileBuilder(resolver, transform,
62+
transform.primaryInput.id, newEntryPointId).run();
63+
} finally {
64+
resolver.release();
65+
}
6666
});
6767
}
6868
});
@@ -93,7 +93,7 @@ class _BootstrapFileBuilder {
9393
new ImportTraversal(_directiveInfo).traverse(entryLib);
9494

9595
var context = new codegen.Context(logger: _transform.logger);
96-
_directiveInfo.initQueue
96+
_directiveInfo.matchQueue
9797
.forEach((entry) => context.directiveRegistry.register(entry));
9898

9999
_transform.addOutput(new Asset.fromString(_newEntryPoint, codegen

modules/angular2/src/transform/traversal.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
library angular2.src.transform;
2+
13
import 'package:analyzer/src/generated/element.dart';
24
import 'package:path/path.dart' as path;
35

modules/angular2/test/transform/common.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
4-
library initialize.test.build.common;
4+
library angular2.test.transform;
55

66
// TODO(kegluneq): Remove this and use the actual Directive def'n.
77
// Simple mock of Directive.

modules/angular2/test/transform/transform_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
library angular2.test;
1+
library angular2.test.transform;
22

33
import 'dart:io';
44
import 'package:barback/barback.dart';

0 commit comments

Comments
 (0)