Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Commit 0784388

Browse files
Pete Bloismhevery
Pete Blois
authored andcommitted
fix(transformer): crashes in metadata extraction while in pub serve
Issue was around how the metadata extractor folds member annotations into the class annotations. It was doing in-place modifications to the AST which would then persist to the next build of the application in pub serve. Correct change is to clone the AST node which is to be modified and leave the original intact. Closes #888
1 parent dafd9e7 commit 0784388

File tree

4 files changed

+58
-4
lines changed

4 files changed

+58
-4
lines changed

lib/tools/source_metadata_extractor.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ class DirectiveMetadataCollectingVisitor {
182182
StringLiteral attNameLiteral = ann.arguments.arguments.first;
183183
if (meta.attributeMappings
184184
.containsKey(attNameLiteral.stringValue)) {
185-
throw 'Attribute mapping already defined for $fieldName';
185+
throw 'Attribute mapping already defined for '
186+
'${clazz.name}.$fieldName';
186187
}
187188
meta.attributeMappings[attNameLiteral.stringValue] =
188189
_attrAnnotationsToSpec[ann.name.name] + fieldName;

lib/tools/transformer/metadata_extractor.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ library angular.metadata_extractor;
22

33
import 'package:analyzer/src/generated/ast.dart';
44
import 'package:analyzer/src/generated/element.dart';
5+
import 'package:analyzer/src/generated/parser.dart' show ResolutionCopier;
56
import 'package:analyzer/src/generated/scanner.dart';
67
import 'package:analyzer/src/generated/utilities_dart.dart' show ParameterKind;
78
import 'package:barback/barback.dart';
@@ -336,7 +337,14 @@ class AnnotationExtractor {
336337

337338
// Default to using the first acceptable annotation- not sure if
338339
// more than one should ever occur.
339-
var annotation = acceptableAnnotations.first;
340+
var sourceAnnotation = acceptableAnnotations.first;
341+
342+
// Clone the annotation so we don't modify the one in the persistent AST.
343+
var index = type.annotations.indexOf(sourceAnnotation);
344+
var annotation = new AstCloner().visitAnnotation(sourceAnnotation);
345+
ResolutionCopier.copyResolutionData(sourceAnnotation, annotation);
346+
type.annotations[index] = annotation;
347+
340348
var mapArg = annotation.arguments.arguments.firstWhere((arg) =>
341349
(arg is NamedExpression) && (arg.name.label.name == 'map'),
342350
orElse: () => null);

test/tools/transformer/expression_generator_spec.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ import 'package:angular/change_detection/change_detection.dart';
181181
''';
182182

183183
const String libAngular = '''
184-
library angular.core.annotation;
184+
library angular.core.annotation_src;
185185
186186
class NgComponent {
187187
const NgComponent({String templateUrl, String selector});

test/tools/transformer/metadata_generator_spec.dart

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,51 @@ main() {
508508
imports: [],
509509
classes: {});
510510
});
511+
512+
it('does not modify annotations in-place', () {
513+
var main = '''
514+
import 'package:angular/angular.dart';
515+
import 'second.dart';
516+
517+
@NgDirective(map: {})
518+
class Engine {
519+
@NgTwoWay('two-way-stuff')
520+
String get twoWayStuff => null;
521+
}
522+
main() {}
523+
''';
524+
return generates(phases,
525+
inputs: {
526+
'angular|lib/angular.dart': libAngular,
527+
'a|web/main.dart': main,
528+
'a|web/second.dart': '''library second;'''
529+
},
530+
imports: [
531+
'import \'main.dart\' as import_0;',
532+
'import \'package:angular/angular.dart\' as import_1;',
533+
],
534+
classes: {
535+
'import_0.Engine': [
536+
'const import_1.NgDirective(map: const {'
537+
'\'two-way-stuff\': \'<=>twoWayStuff\'})',
538+
]
539+
}).then((_) => generates(phases,
540+
inputs: {
541+
'angular|lib/angular.dart': libAngular,
542+
'a|web/main.dart': main,
543+
'a|web/second.dart': '''library a.second;'''
544+
},
545+
imports: [
546+
'import \'main.dart\' as import_0;',
547+
'import \'package:angular/angular.dart\' as import_1;',
548+
],
549+
classes: {
550+
'import_0.Engine': [
551+
'const import_1.NgDirective(map: const {'
552+
'\'two-way-stuff\': \'<=>twoWayStuff\'})',
553+
]
554+
}));
555+
});
511556
});
512557
}
513558

@@ -568,7 +613,7 @@ const String footer = '''
568613

569614

570615
const String libAngular = '''
571-
library angular.core.annotation;
616+
library angular.core.annotation_src;
572617
573618
class AbstractNgAnnotation {
574619
AbstractNgAnnotation({map: const {}});

0 commit comments

Comments
 (0)