Skip to content

Commit

Permalink
fix(dart/transformer): Correctly handle const object annotations
Browse files Browse the repository at this point in the history
Previously, annotations which were const objects (as opposed to const
instance creation expressions) were incorrectly output as instance
creation expressions.

Before:
```
const override() // A const instance creation expression
```

After
```
override // A const instance
```

Closes #4481
  • Loading branch information
Tim Blasi committed Oct 3, 2015
1 parent 01fb06a commit decdbea
Show file tree
Hide file tree
Showing 9 changed files with 216 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class _ExtractQueryFieldsFromPropMetadata extends Object
bool _hasQueryAnnotation(list) {
var res = false;
list.elements.forEach((item) {
if (item is! InstanceCreationExpression) return;
var n = item.constructorName.toString();
if (n == "ContentChild" ||
n == "ViewChild" ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,12 @@ class AnnotationVisitor extends SimpleAstVisitor<AnnotationModel> {
..isComponent = isComponent
..isDirective = isDirective
..isInjectable = isInjectable
..isView = isView;
..isView = isView
..isConstObject = node.arguments == null;

if (node.arguments != null) {
// This annotation is a constant instance creation expression,
// e.g. @Injectable(), rather than a const object, e.g. @override.
if (!model.isConstObject) {
for (var arg in node.arguments.arguments) {
if (arg is NamedExpression) {
model.namedParameters.add(new NamedParameter()
Expand All @@ -60,7 +63,11 @@ abstract class AnnotationWriterMixin {
StringBuffer get buffer;

void writeAnnotationModel(AnnotationModel model) {
if (model.parameters != null || model.namedParameters != null) {
if (model.isConstObject) {
// This is a const instance, not a ctor invocation and does not need a
// const instance creation expression.
buffer.write(model.name);
} else {
buffer.write('const ${model.name}(');
var first = true;
for (var param in model.parameters) {
Expand All @@ -83,10 +90,6 @@ abstract class AnnotationWriterMixin {
buffer.write('${param.name}: ${param.value}');
}
buffer.write(')');
} else {
// This is a const instance, not a ctor invocation and does not need a
// const instance creation expression.
buffer.write(model.name);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,67 +8,91 @@ import 'package:protobuf/protobuf.dart';
class NamedParameter extends GeneratedMessage {
static final BuilderInfo _i = new BuilderInfo('NamedParameter')
..a(1, 'name', PbFieldType.QS)
..a(2, 'value', PbFieldType.QS)
;
..a(2, 'value', PbFieldType.QS);

NamedParameter() : super();
NamedParameter.fromBuffer(List<int> i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r);
NamedParameter.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r);
NamedParameter.fromBuffer(List<int> i,
[ExtensionRegistry r = ExtensionRegistry.EMPTY])
: super.fromBuffer(i, r);
NamedParameter.fromJson(String i,
[ExtensionRegistry r = ExtensionRegistry.EMPTY])
: super.fromJson(i, r);
NamedParameter clone() => new NamedParameter()..mergeFromMessage(this);
BuilderInfo get info_ => _i;
static NamedParameter create() => new NamedParameter();
static PbList<NamedParameter> createRepeated() => new PbList<NamedParameter>();
static PbList<NamedParameter> createRepeated() =>
new PbList<NamedParameter>();
static NamedParameter getDefault() {
if (_defaultInstance == null) _defaultInstance = new _ReadonlyNamedParameter();
if (_defaultInstance == null) _defaultInstance =
new _ReadonlyNamedParameter();
return _defaultInstance;
}

static NamedParameter _defaultInstance;
static void $checkItem(NamedParameter v) {
if (v is !NamedParameter) checkItemFailed(v, 'NamedParameter');
if (v is! NamedParameter) checkItemFailed(v, 'NamedParameter');
}

String get name => getField(1);
void set name(String v) { setField(1, v); }
void set name(String v) {
setField(1, v);
}

bool hasName() => hasField(1);
void clearName() => clearField(1);

String get value => getField(2);
void set value(String v) { setField(2, v); }
void set value(String v) {
setField(2, v);
}

bool hasValue() => hasField(2);
void clearValue() => clearField(2);
}

class _ReadonlyNamedParameter extends NamedParameter with ReadonlyMessageMixin {}
class _ReadonlyNamedParameter extends NamedParameter with ReadonlyMessageMixin {
}

class AnnotationModel extends GeneratedMessage {
static final BuilderInfo _i = new BuilderInfo('AnnotationModel')
..a(1, 'name', PbFieldType.QS)
..p(2, 'parameters', PbFieldType.PS)
..pp(3, 'namedParameters', PbFieldType.PM, NamedParameter.$checkItem, NamedParameter.create)
..pp(3, 'namedParameters', PbFieldType.PM, NamedParameter.$checkItem,
NamedParameter.create)
..a(4, 'isView', PbFieldType.OB)
..a(5, 'isDirective', PbFieldType.OB)
..a(6, 'isComponent', PbFieldType.OB)
..a(7, 'isInjectable', PbFieldType.OB)
;
..a(8, 'isConstObject', PbFieldType.OB);

AnnotationModel() : super();
AnnotationModel.fromBuffer(List<int> i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r);
AnnotationModel.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r);
AnnotationModel.fromBuffer(List<int> i,
[ExtensionRegistry r = ExtensionRegistry.EMPTY])
: super.fromBuffer(i, r);
AnnotationModel.fromJson(String i,
[ExtensionRegistry r = ExtensionRegistry.EMPTY])
: super.fromJson(i, r);
AnnotationModel clone() => new AnnotationModel()..mergeFromMessage(this);
BuilderInfo get info_ => _i;
static AnnotationModel create() => new AnnotationModel();
static PbList<AnnotationModel> createRepeated() => new PbList<AnnotationModel>();
static PbList<AnnotationModel> createRepeated() =>
new PbList<AnnotationModel>();
static AnnotationModel getDefault() {
if (_defaultInstance == null) _defaultInstance = new _ReadonlyAnnotationModel();
if (_defaultInstance == null) _defaultInstance =
new _ReadonlyAnnotationModel();
return _defaultInstance;
}

static AnnotationModel _defaultInstance;
static void $checkItem(AnnotationModel v) {
if (v is !AnnotationModel) checkItemFailed(v, 'AnnotationModel');
if (v is! AnnotationModel) checkItemFailed(v, 'AnnotationModel');
}

String get name => getField(1);
void set name(String v) { setField(1, v); }
void set name(String v) {
setField(1, v);
}

bool hasName() => hasField(1);
void clearName() => clearField(1);

Expand All @@ -77,27 +101,48 @@ class AnnotationModel extends GeneratedMessage {
List<NamedParameter> get namedParameters => getField(3);

bool get isView => getField(4);
void set isView(bool v) { setField(4, v); }
void set isView(bool v) {
setField(4, v);
}

bool hasIsView() => hasField(4);
void clearIsView() => clearField(4);

bool get isDirective => getField(5);
void set isDirective(bool v) { setField(5, v); }
void set isDirective(bool v) {
setField(5, v);
}

bool hasIsDirective() => hasField(5);
void clearIsDirective() => clearField(5);

bool get isComponent => getField(6);
void set isComponent(bool v) { setField(6, v); }
void set isComponent(bool v) {
setField(6, v);
}

bool hasIsComponent() => hasField(6);
void clearIsComponent() => clearField(6);

bool get isInjectable => getField(7);
void set isInjectable(bool v) { setField(7, v); }
void set isInjectable(bool v) {
setField(7, v);
}

bool hasIsInjectable() => hasField(7);
void clearIsInjectable() => clearField(7);

bool get isConstObject => getField(8);
void set isConstObject(bool v) {
setField(8, v);
}

bool hasIsConstObject() => hasField(8);
void clearIsConstObject() => clearField(8);
}

class _ReadonlyAnnotationModel extends AnnotationModel with ReadonlyMessageMixin {}
class _ReadonlyAnnotationModel extends AnnotationModel
with ReadonlyMessageMixin {}

const NamedParameter$json = const {
'1': 'NamedParameter',
Expand All @@ -112,17 +157,24 @@ const AnnotationModel$json = const {
'2': const [
const {'1': 'name', '3': 1, '4': 2, '5': 9},
const {'1': 'parameters', '3': 2, '4': 3, '5': 9},
const {'1': 'named_parameters', '3': 3, '4': 3, '5': 11, '6': '.angular2.src.transform.common.model.proto.NamedParameter'},
const {
'1': 'named_parameters',
'3': 3,
'4': 3,
'5': 11,
'6': '.angular2.src.transform.common.model.proto.NamedParameter'
},
const {'1': 'is_view', '3': 4, '4': 1, '5': 8},
const {'1': 'is_directive', '3': 5, '4': 1, '5': 8},
const {'1': 'is_component', '3': 6, '4': 1, '5': 8},
const {'1': 'is_injectable', '3': 7, '4': 1, '5': 8},
const {'1': 'is_const_object', '3': 8, '4': 1, '5': 8},
],
};

/**
* Generated with:
* annotation_model.proto (5ca037df4c4d3e5d2771a177c9f5ea9dc8ae5f91)
* annotation_model.proto (93cb7c1fba2e56d937fec054b6e119a2a2b9afe7)
* libprotoc 2.5.0
* dart-protoc-plugin (cc35f743de982a4916588b9c505dd21c7fe87d17)
*/
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,16 @@ message AnnotationModel {
// account, that is, this should be true if `is_component` is true.
optional bool is_directive = 5;

// Whether htis is a `Component` annotation.
// Whether this is a `Component` annotation.
optional bool is_component = 6;

// Whether this is an `Injectable` annotation. This takes inheritance into
// account, that is, this should be true if `is_directive` and/or
// `is_component` is true.
optional bool is_injectable = 7;

// Whether this annotation is a constant object (for example, `@override`) as
// opposed to a const instance creation expression
// (for example, `@Injectable()`).
optional bool is_const_object = 8;
}
62 changes: 39 additions & 23 deletions modules_dart/transform/test/transform/bind_generator/all_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,49 +16,65 @@ void allTests() {

it('should generate a setter for an `inputs` property in an annotation.',
() async {
var inputPath = 'bind_generator/basic_bind_files/bar.ng_deps.dart';
var expected = formatter.format(
readFile('bind_generator/basic_bind_files/expected/bar.ng_deps.dart'));
var inputPath = 'basic_bind_files/bar.ng_deps.dart';
var expected = _readFile('basic_bind_files/expected/bar.ng_deps.dart');

var output = formatter.format(
await createNgSettersAndGetters(reader, new AssetId('a', inputPath)));
var output = formatter
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected);
});

it(
'should generate a single setter when multiple annotations bind to the '
'same `inputs` property.', () async {
var inputPath =
'bind_generator/duplicate_bind_name_files/soup.ng_deps.dart';
var expected = formatter.format(readFile(
'bind_generator/duplicate_bind_name_files/expected/soup.ng_deps.dart'));
var inputPath = 'duplicate_bind_name_files/soup.ng_deps.dart';
var expected =
_readFile('duplicate_bind_name_files/expected/soup.ng_deps.dart');

var output = formatter.format(
await createNgSettersAndGetters(reader, new AssetId('a', inputPath)));
var output = formatter
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected);
});

it('should generate setters for queries defined in the class annotation.',
() async {
var inputPath =
'bind_generator/queries_class_annotation_files/bar.ng_deps.dart';
var expected = formatter.format(readFile(
'bind_generator/queries_class_annotation_files/expected/bar.ng_deps.dart'));
var inputPath = 'queries_class_annotation_files/bar.ng_deps.dart';
var expected =
_readFile('queries_class_annotation_files/expected/bar.ng_deps.dart');

var output = formatter.format(
await createNgSettersAndGetters(reader, new AssetId('a', inputPath)));
var output = formatter
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected);
});

it('should generate setters for queries defined via prop annotations.',
() async {
var inputPath =
'bind_generator/queries_prop_annotations_files/bar.ng_deps.dart';
var expected = formatter.format(readFile(
'bind_generator/queries_prop_annotations_files/expected/bar.ng_deps.dart'));
var inputPath = 'queries_prop_annotations_files/bar.ng_deps.dart';
var expected =
_readFile('queries_prop_annotations_files/expected/bar.ng_deps.dart');

var output = formatter.format(
await createNgSettersAndGetters(reader, new AssetId('a', inputPath)));
var output = formatter
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected);
});

it('should gracefully handle const objects as prop annotations.', () async {
var inputPath = 'queries_override_annotation_files/bar.ng_deps.dart';
var expected = formatter.format(_readFile(
'queries_override_annotation_files/expected/bar.ng_deps.dart'));

var output = formatter
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected);
});
}

AssetId _assetId(String path) => new AssetId('a', 'bind_generator/$path');

String _readFile(String path) {
var code = readFile('bind_generator/$path');
if (path.endsWith('.dart')) {
code = formatter.format(code);
}
return code;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
library bar.ng_deps.dart;

import 'bar.dart';
import 'package:angular2/src/core/metadata.dart';

var _visited = false;
void initReflector(reflector) {
if (_visited) return;
_visited = true;
reflector
..registerType(
ToolTip,
new ReflectionInfo(
const [const Directive(selector: '[tool-tip]')],
const [],
() => new ToolTip(),
null,
const {
'queryField': const [override]
}));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
library bar.ng_deps.dart;

import 'bar.dart';
import 'package:angular2/src/core/metadata.dart';

var _visited = false;
void initReflector(reflector) {
if (_visited) return;
_visited = true;
reflector
..registerType(
ToolTip,
new ReflectionInfo(
const [const Directive(selector: '[tool-tip]')],
const [],
() => new ToolTip(),
null,
const {
'queryField': const [override]
}));
}
Loading

0 comments on commit decdbea

Please sign in to comment.