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

fix(dynamic_parser): Handle reserved words correctly. #614

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
200 changes: 199 additions & 1 deletion bin/parser_generator_for_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,204 @@ main(arguments) {
"_privateField",
"'World'|hello",
"1;'World'|hello",
"'World'|hello;1"
"'World'|hello;1",

"assert",
"break",
"case",
"catch",
"class",
"const",
"continue",
"default",
"do",
"else",
"enum",
"extends",
"final",
"finally",
"for",
"if",
"in",
"is",
"new",
"rethrow",
"return",
"super",
"switch",
"this",
"throw",
"try",
"var",
"void",
"while",
"with",

"assert = 42",
"break = 42",
"case = 42",
"catch = 42",
"class = 42",
"const = 42",
"continue = 42",
"default = 42",
"do = 42",
"else = 42",
"enum = 42",
"extends = 42",
"false = 42",
"final = 42",
"finally = 42",
"for = 42",
"if = 42",
"in = 42",
"is = 42",
"new = 42",
"null = 42",
"rethrow = 42",
"return = 42",
"super = 42",
"switch = 42",
"this = 42",
"throw = 42",
"true = 42",
"try = 42",
"var = 42",
"void = 42",
"while = 42",
"with = 42",

"assert()",
"break()",
"case()",
"catch()",
"class()",
"const()",
"continue()",
"default()",
"do()",
"else()",
"enum()",
"extends()",
"final()",
"finally()",
"for()",
"if()",
"in()",
"is()",
"new()",
"rethrow()",
"return()",
"super()",
"switch()",
"this()",
"throw()",
"try()",
"var()",
"void()",
"while()",
"with()",

"o.assert",
"o.break",
"o.case",
"o.catch",
"o.class",
"o.const",
"o.continue",
"o.default",
"o.do",
"o.else",
"o.enum",
"o.extends",
"o.false",
"o.final",
"o.finally",
"o.for",
"o.if",
"o.in",
"o.is",
"o.new",
"o.null",
"o.rethrow",
"o.return",
"o.super",
"o.switch",
"o.this",
"o.throw",
"o.true",
"o.try",
"o.var",
"o.void",
"o.while",
"o.with",

"o.assert = 42",
"o.break = 42",
"o.case = 42",
"o.catch = 42",
"o.class = 42",
"o.const = 42",
"o.continue = 42",
"o.default = 42",
"o.do = 42",
"o.else = 42",
"o.enum = 42",
"o.extends = 42",
"o.false = 42",
"o.final = 42",
"o.finally = 42",
"o.for = 42",
"o.if = 42",
"o.in = 42",
"o.is = 42",
"o.new = 42",
"o.null = 42",
"o.rethrow = 42",
"o.return = 42",
"o.super = 42",
"o.switch = 42",
"o.this = 42",
"o.throw = 42",
"o.true = 42",
"o.try = 42",
"o.var = 42",
"o.void = 42",
"o.while = 42",
"o.with = 42",

"o.assert()",
"o.break()",
"o.case()",
"o.catch()",
"o.class()",
"o.const()",
"o.continue()",
"o.default()",
"o.do()",
"o.else()",
"o.enum()",
"o.extends()",
"o.false()",
"o.final()",
"o.finally()",
"o.for()",
"o.if()",
"o.in()",
"o.is()",
"o.new()",
"o.null()",
"o.rethrow()",
"o.return()",
"o.super()",
"o.switch()",
"o.this()",
"o.throw()",
"o.true()",
"o.try()",
"o.var()",
"o.void()",
"o.while()",
"o.with()",
]);
}
9 changes: 6 additions & 3 deletions lib/core/parser/eval_access.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'package:angular/core/module.dart';

class AccessScope extends syntax.AccessScope with AccessReflective {
final Symbol symbol;
AccessScope(String name) : super(name), symbol = new Symbol(name);
AccessScope(String name) : super(name), symbol = newSymbol(name);
eval(scope, [FilterMap filters]) => _eval(scope);
assign(scope, value) => _assign(scope, scope, value);
}
Expand All @@ -24,7 +24,7 @@ class AccessScopeFast extends syntax.AccessScope with AccessFast {
class AccessMember extends syntax.AccessMember with AccessReflective {
final Symbol symbol;
AccessMember(object, String name)
: super(object, name), symbol = new Symbol(name);
: super(object, name), symbol = newSymbol(name);
eval(scope, [FilterMap filters]) => _eval(object.eval(scope, filters));
assign(scope, value) => _assign(scope, object.eval(scope), value);
_assignToNonExisting(scope, value) => object.assign(scope, { name: value });
Expand Down Expand Up @@ -84,6 +84,9 @@ abstract class AccessReflective {
_cachedKind = CACHED_MAP;
_cachedValue = null;
return holder[name];
} else if (symbol == null) {
_cachedHolder = UNINITIALIZED;
return null;
}
InstanceMirror mirror = reflect(holder);
try {
Expand Down Expand Up @@ -119,7 +122,7 @@ abstract class AccessReflective {
holder[name] = value;
} else if (holder == null) {
_assignToNonExisting(scope, value);
} else {
} else if (symbol != null) {
reflect(holder).setField(symbol, value);
}
return value;
Expand Down
7 changes: 5 additions & 2 deletions lib/core/parser/eval_calls.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ class CallScope extends syntax.CallScope with CallReflective {
final Symbol symbol;
CallScope(name, arguments)
: super(name, arguments)
, symbol = new Symbol(name);
, symbol = newSymbol(name);
eval(scope, [FilterMap filters]) => _eval(scope, scope);
}

class CallMember extends syntax.CallMember with CallReflective {
final Symbol symbol;
CallMember(object, name, arguments)
: super(object, name, arguments)
, symbol = new Symbol(name);
, symbol = newSymbol(name);
eval(scope, [FilterMap filters]) => _eval(scope, object.eval(scope, filters));
}

Expand Down Expand Up @@ -94,6 +94,9 @@ abstract class CallReflective {
_cachedKind = CACHED_MAP;
_cachedValue = null;
return relaxFnApply(ensureFunctionFromMap(holder, name), arguments);
} else if (symbol == null) {
_cachedHolder = UNINITIALIZED;
throw new EvalError("Undefined function $name");
} else {
InstanceMirror mirror = reflect(holder);
_cachedKind = CACHED_FUNCTION;
Expand Down
9 changes: 8 additions & 1 deletion lib/core/parser/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ library angular.core.parser.utils;

import 'package:angular/core/parser/syntax.dart' show Expression;
import 'package:angular/core/module.dart';
import 'package:angular/utils.dart' show isReservedWord;
export 'package:angular/utils.dart' show relaxFnApply, relaxFnArgs, toBool;

/// Marker for an uninitialized value.
Expand All @@ -21,7 +22,7 @@ class EvalError {
/// Evaluate the [list] in context of the [scope].
List evalList(scope, List<Expression> list, [FilterMap filters]) {
int length = list.length;
for(int cacheLength = _evalListCache.length; cacheLength <= length; cacheLength++) {
for (int cacheLength = _evalListCache.length; cacheLength <= length; cacheLength++) {
_evalListCache.add(new List(cacheLength));
}
List result = _evalListCache[length];
Expand Down Expand Up @@ -96,3 +97,9 @@ setKeyed(object, key, value) {
}
return value;
}

/// Returns a new symbol with the given name if the name is a legal
Copy link
Contributor

Choose a reason for hiding this comment

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

This doc comment opens the question as to what it means for a name to be "legal". Maybe drop the term "legal" and just refer to names not being reserved words.

/// symbol name. Otherwise, returns null.
Symbol newSymbol(String name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This choice of "newSymbol" name for this convenience function seems ... slightly misleading (if one doesn't read the doc comment). Maybe newNonReservedWordSymbol() ... yes, longer, but certainly clear. At least newNrwSymbol() will entice a developer to read the API doc rather than just assume newSymbol() returns a new symbol. My 2 cents.

return isReservedWord(name) ? null : new Symbol(name);
}
1 change: 0 additions & 1 deletion lib/core_dom/module.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import 'package:perf_api/perf_api.dart';

import 'package:angular/core/module.dart';
import 'package:angular/core/parser/parser.dart';
import 'package:angular/utils.dart';

part 'block.dart';
part 'block_factory.dart';
Expand Down
16 changes: 13 additions & 3 deletions lib/directive/ng_pluralize.dart
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,16 @@ class NgPluralizeDirective {
int offset;
var discreteRules = <String, String>{};
var categoryRules = <Symbol, String>{};

static final RegExp IS_WHEN = new RegExp(r'^when-(minus-)?.');
static const Map<String, Symbol> SYMBOLS = const {
'zero' : #zero,
'one' : #one,
'two' : #two,
'few' : #few,
'many' : #many,
'other' : #other,
};

NgPluralizeDirective(this.scope, this.element, this.interpolate,
NodeAttrs attributes, this.parser) {
Expand All @@ -116,10 +125,11 @@ class NgPluralizeDirective {
}

whens.forEach((k, v) {
if (['zero', 'one', 'two', 'few', 'many', 'other'].contains(k)) {
this.categoryRules[new Symbol(k.toString())] = v;
Symbol symbol = SYMBOLS[k];
if (symbol != null) {
this.categoryRules[symbol] = v;
} else {
this.discreteRules[k.toString()] = v;
this.discreteRules[k] = v;
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions lib/tools/parser_generator/dart_code_gen.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
library dart_code_gen;

import 'package:angular/tools/reserved_dart_keywords.dart';
import 'package:angular/utils.dart' show isReservedWord;
import 'package:angular/core/parser/syntax.dart';

escape(String s) => s.replaceAllMapped(new RegExp(r'(\"|\$|\n)'), (m) {
Expand Down Expand Up @@ -229,7 +229,7 @@ class HelperMap {
String lookup(String key) {
String name = _computeName(key);
if (helpers.containsKey(key)) return name;
helpers[key] = isReserved(key)
helpers[key] = isReservedWord(key)
? templateForReserved(name, key)
: template(name, key);
return name;
Expand Down
6 changes: 3 additions & 3 deletions lib/tools/parser_getter_setter/generator.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import 'package:angular/core/parser/parser.dart';
import 'package:angular/tools/reserved_dart_keywords.dart';
import 'package:angular/utils.dart' show isReservedWord;
import 'dart:math';

class DartGetterSetterGen extends ParserBackend {
Expand All @@ -9,12 +9,12 @@ class DartGetterSetterGen extends ParserBackend {
bool isAssignable(expression) => true;

registerAccess(String name) {
if (isReserved(name)) return;
if (isReservedWord(name)) return;
properties.add(name);
}

registerCall(String name, List arguments) {
if (isReserved(name)) return;
if (isReservedWord(name)) return;
Set<int> arities = calls.putIfAbsent(name, () => new Set<int>());
arities.add(arguments.length);
}
Expand Down
10 changes: 0 additions & 10 deletions lib/tools/reserved_dart_keywords.dart

This file was deleted.

1 change: 0 additions & 1 deletion lib/tools/source_metadata_extractor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import 'package:analyzer/src/generated/ast.dart';

import 'package:angular/tools/source_crawler.dart';
import 'package:angular/tools/common.dart';
import 'package:angular/utils.dart';

const String _COMPONENT = '-component';
const String _DIRECTIVE = '-directive';
Expand Down
Loading