Add full scoping support for Library #11
Conversation
yjbanov
left a comment
yjbanov
left a comment
There was a problem hiding this comment.
Up to you if you want to use Dart keywords as identifiers. I'd be worried that one day it might become illegal as the language evolves. Also, it throws off syntax highlighters.
My suggestion is to be one standard prefix for all booleans that control the existence of a keyword. For example "as":
new FunctionBuilder(
asAbstract: true,
asStatic: false,
)
lib/code_builder.dart
Outdated
| @visibleForTesting | ||
| String dartfmt(String source) => _dartfmt.format(source); | ||
|
|
||
| // Creates a defensive copy of an AST node. |
There was a problem hiding this comment.
Is cloning only useful for defensive purposes? If not, I'd drop "defensive". Also, if the cloning is deep, I'd note that.
There was a problem hiding this comment.
defensive --> deep.
Done.
lib/code_builder.dart
Outdated
| return new SimpleIdentifier(new StringToken(TokenType.STRING, s, 0)); | ||
| } | ||
|
|
||
| Literal _stringLit(String s) { |
There was a problem hiding this comment.
dart style police calls: avoid abbreviations
| } | ||
|
|
||
| // Simplifies some of the builders by having a mutable node we clone from. | ||
| @Deprecated('Builders are all becoming lazy') |
There was a problem hiding this comment.
Is it worth deprecating an internal class?
There was a problem hiding this comment.
It's helping me (in the analyzer) show what work I have left to do
lib/src/builders/class_builder.dart
Outdated
| }) => | ||
| new ClassBuilder._( | ||
| name, | ||
| abstract, |
There was a problem hiding this comment.
While legal Dart code I'm not a fan of Dart keywords used as identifiers, especially in the public API (this named parameter is visible to user code)
There was a problem hiding this comment.
Done.
Split off to ClassBuilder.asAbstract(...)
|
|
||
| /// Returns wrapped as a [FunctionExpression] AST. | ||
| FunctionExpression toFunctionExpression() => _asFunctionExpression(this); | ||
| FunctionExpression toFunctionExpression([Scope scope = const Scope.identity()]) => _asFunctionExpression(this, scope); |
There was a problem hiding this comment.
This is a looooong line. Did you dartfmt this file?
lib/src/builders/field_builder.dart
Outdated
| this._name, { | ||
| TypeBuilder type, | ||
| ExpressionBuilder initialize, | ||
| bool static: false, |
There was a problem hiding this comment.
--> asStatic. Thanks!
* Add full scoping support for Library * Address comments.
3 participants
Also ran dartfmt and organized members by name in classes so the diff is much larger than it actually is (sorry about that). Added more tests for when a
Scopeis used.