Skip to content
This repository was archived by the owner on Apr 8, 2025. It is now read-only.

Add additional features for NG2 output AST parity #29

Merged
matanlurey merged 5 commits into from
Nov 17, 2016
Merged

Add additional features for NG2 output AST parity #29

matanlurey merged 5 commits into from
Nov 17, 2016

Conversation

@matanlurey
Copy link
Contributor

@matanlurey matanlurey commented Nov 17, 2016

Closes dart-lang/tools#956
Closes dart-lang/tools#957

1.0.0-alpha+3

  • BREAKING CHANGE: Added generics support to TypeBuilder:

importFrom becomes a named, not positional argument, and the named
argument genericTypes is added (Iterable<TypeBuilder>).

// List<String>
new TypeBuilder('List', genericTypes: [reference('String')])
  • Added generic support to ReferenceBuilder:
// List<String>
reference('List').toTyped([reference('String')])
  • Fixed a bug where ReferenceBuilder.buildAst was not implemented
  • Added and and or methods to ExpressionBuilder:
// true || false
literal(true).or(literal(false));

// true && false
literal(true).and(literal(false));
  • Added support for creating closures - MethodBuilder.closure:
// () => true
new MethodBuilder.closure(
  returns: literal(true),
  returnType: lib$core.bool,
)

alorenzen
alorenzen reviewed Nov 17, 2016
View reviewed changes
lib/src/builders/method.dart
@override
AstNode buildAst([Scope scope]) => buildFunction(scope);
AstNode buildAst([Scope scope]) {
if (_name != null) {
Copy link
Contributor

@alorenzen alorenzen Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More of a personal style, but I like to use => and ternary operator here:
buildAst() => _name != null ? buildFunction() : buildExpression();

Copy link
Contributor Author

@matanlurey matanlurey Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly just because dartfmt doesn't do a great job with > 80 char lambdas.

I can change it in a future PR

Copy link
Contributor

@alorenzen alorenzen Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. Like I said, it's just my personal preference.

lib/src/builders/method.dart

@override
Expression buildExpression([Scope scope]) {
return new FunctionExpression(
Copy link
Contributor

@alorenzen alorenzen Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just use => ?

Copy link
Contributor Author

@matanlurey matanlurey Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

lib/src/builders/reference.dart
String name,
String importFrom,
)
: super._(name, importFrom);
Copy link
Contributor

@alorenzen alorenzen Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation seems off here. Dartfmt?

Copy link
Contributor Author

@matanlurey matanlurey Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is dartfmt :-/

pubspec.yaml
@@ -1,5 +1,5 @@
name: code_builder
version: 1.0.0-alpha+2
version: 1.0.0-alpha+3
Copy link
Contributor

@alorenzen alorenzen Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure about the standard practice here, but should we bump the version # given a breaking change? Or because this is all still alpha, we're not too worried?

Copy link
Contributor Author

@matanlurey matanlurey Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm basically making the assumption anyone using -alpha is using an experimental branch - not worth the churn over making this version 33.0.0 by the time we are done.

Copy link
Contributor

@alorenzen alorenzen Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

alorenzen
alorenzen approved these changes Nov 17, 2016
View reviewed changes
Copy link
Contributor

@alorenzen alorenzen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matanlurey matanlurey merged commit e61c9a1 into dart-archive:master Nov 17, 2016
@matanlurey matanlurey deleted the more_ng2_parity branch November 17, 2016 01:39
mosuem pushed a commit to dart-lang/tools that referenced this pull request Oct 25, 2024
@matanlurey
Add additional features for NG2 output AST parity (dart-archive/code_…
721c909 
…builder#29)

* Various changes.

* Update presubmit.

* Add more features required for ng2 parity.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@alorenzen alorenzen alorenzen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support generic types Need a ClosureBuilder

3 participants

@matanlurey @alorenzen @googlebot