Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New features for 1.0.0-beta #52

Merged
merged 7 commits into from Jan 5, 2017
Merged

Conversation

matanlurey
Copy link
Contributor

I don't expect anyone to get to this before the new year

Closes #50
Closes #49
Closes #47

Partial support on #43

1.0.0-beta

  • Add support for async, sync, sync* functions
  • Add support for expression asAwait, asYield, asYieldStar
  • Add toExportBuilder and toImportBuilder to types and references
  • Fix an import scoping bug in return statements and named constructor invocations.
  • Added constructor initializer support
  • Add while and do {} while loop support
  • Add for and for-in support
  • Added a name getter for ParameterBuilder

/cc @alorenzen

Copy link
Member

@natebosch natebosch left a comment

Choose a reason for hiding this comment

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

Tests for increment and decrement?

@@ -210,8 +240,13 @@ abstract class AbstractExpressionMixin implements ExpressionBuilder {
}

@override
ExpressionBuilder increment([bool prefix = false]) {
return new _IncrementExpression(this, prefix);
Copy link
Member

Choose a reason for hiding this comment

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

[nit] how are you choosing between => and return? Why is this one different from decrement above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying not to use => when it creates a new line, but probably inconsistent.

ExpressionBuilder identical(ExpressionBuilder other) {
return lib$core.identical.call([
return lib$core.identical.call(<ExpressionBuilder>[
Copy link
Member

Choose a reason for hiding this comment

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

is this fixing an analyzer warning? I'd be surprised if this wasn't inferred in strong mode...

Copy link
Contributor Author

@matanlurey matanlurey Jan 3, 2017

Choose a reason for hiding this comment

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

Yes, it could not infer the type and warned as such.

@@ -186,7 +186,18 @@ abstract class ConstructorBuilder
HasStatements,
ValidClassMember {
/// Create a new [ConstructorBuilder], optionally with a [name].
Copy link
Member

Choose a reason for hiding this comment

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

Doc comment is no longer complete. This is a case where we can add useful information explaining the 'super' arguments ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged.

List<ExpressionBuilder> invokeSuper,
}) = _NormalConstructorBuilder;

/// Adds a constructor initializer.
Copy link
Member

Choose a reason for hiding this comment

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

[nit] 'constructor initializer' reads a bit strange to me... how about

/// Adds a field initializer to this constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -47,6 +47,9 @@ final Token $deferred = new KeywordToken(Keyword.DEFERRED, 0);
/// The `/` token.
final Token $divide = new Token(TokenType.SLASH, 0);

/// The `do` keyword.
Copy link
Member

Choose a reason for hiding this comment

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

can we consider dropping this lint? The zero-information comments are very noisy throughout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make it consistent I'll drop them all in another PR.

@@ -95,4 +95,48 @@ void main() {
);
});
});

group('for statemnets', () {
Copy link
Member

Choose a reason for hiding this comment

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

[nit] typo in statements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@natebosch natebosch left a comment

Choose a reason for hiding this comment

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

Consider adding a test for increment/decrement in a followup

@matanlurey matanlurey merged commit b2d01d4 into dart-lang:master Jan 5, 2017
@matanlurey matanlurey deleted the new-features branch January 5, 2017 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants