Skip to content

Commit 02a467b

Browse files
srawlinssrawlins
authored
Report errors on SimpleRenderer on missing getters (#2670)
1 parent b6fe5dd commit 02a467b

File tree

8 files changed

+1219
-229
lines changed

8 files changed

+1219
-229
lines changed

‎lib/src/generator/templates.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ const _visibleTypes = {
4747
Constructor,
4848
DefinedElementType,
4949
Documentable,
50+
ElementType,
5051
Enum,
5152
Extension,
5253
FeatureSet,

‎lib/src/generator/templates.runtime_renderers.dart‎

Lines changed: 1002 additions & 187 deletions
Large diffs are not rendered by default.

‎lib/src/mustachio/renderer_base.dart‎

Lines changed: 70 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,14 @@ abstract class RendererBase<T> {
147147
/// The output buffer into which [context] is rendered, using a template.
148148
final buffer = StringBuffer();
149149

150-
RendererBase(this.context, this.parent, this._template);
150+
final Set<String> _invisibleGetters;
151+
152+
RendererBase(
153+
this.context,
154+
this.parent,
155+
this._template, {
156+
Set<String> invisibleGetters = const {},
157+
}) : _invisibleGetters = invisibleGetters;
151158

152159
Template get template => _template;
153160

@@ -172,21 +179,28 @@ abstract class RendererBase<T> {
172179
if (names.length == 1 && names.single == '.') {
173180
return context.toString();
174181
}
175-
var property = getProperty(names.first);
176-
if (property != null) {
177-
var remainingNames = [...names.skip(1)];
178-
try {
179-
return property.renderVariable(context, property, remainingNames);
180-
} on PartialMustachioResolutionError catch (e) {
181-
// The error thrown by [Property.renderVariable] does not have all of
182-
// the names required for a decent error. We throw a new error here.
183-
throw MustachioResolutionError(node.keySpan.message(
184-
"Failed to resolve '${e.name}' on ${e.contextType} while resolving "
185-
'$remainingNames as a property chain on any types in the context '
186-
"chain: $contextChainString, after first resolving '${names.first}' "
187-
'to a property on $T'));
182+
var firstName = names.first;
183+
try {
184+
var property = getProperty(firstName);
185+
if (property != null) {
186+
var remainingNames = [...names.skip(1)];
187+
try {
188+
return property.renderVariable(context, property, remainingNames);
189+
} on PartialMustachioResolutionError catch (e) {
190+
// The error thrown by [Property.renderVariable] does not have all of
191+
// the names required for a decent error. We throw a new error here.
192+
throw MustachioResolutionError(node.keySpan.message(
193+
"Failed to resolve '${e.name}' on ${e.contextType} while "
194+
'resolving $remainingNames as a property chain on any types in '
195+
'the context chain: $contextChainString, after first resolving '
196+
"'$firstName' to a property on $T"));
197+
}
188198
}
189-
} else if (parent != null) {
199+
} on _MustachioResolutionErrorWithoutSpan catch (e) {
200+
throw MustachioResolutionError(node.keySpan.message(e.message));
201+
}
202+
203+
if (parent != null) {
190204
return parent.getFields(node);
191205
} else {
192206
throw MustachioResolutionError(node.keySpan.message(
@@ -268,31 +282,52 @@ abstract class RendererBase<T> {
268282
}
269283

270284
String renderSimple(Object context, List<MustachioNode> ast, Template template,
271-
{RendererBase parent}) {
272-
var renderer = SimpleRenderer(context, parent, template);
285+
{@required RendererBase parent, Set<String> getters}) {
286+
var renderer = SimpleRenderer(context, parent, template, getters);
273287
renderer.renderBlock(ast);
274288
return renderer.buffer.toString();
275289
}
276290

277291
class SimpleRenderer extends RendererBase<Object> {
278-
SimpleRenderer(Object context, RendererBase<Object> parent, Template template)
279-
: super(context, parent, template);
292+
SimpleRenderer(
293+
Object context,
294+
RendererBase<Object> parent,
295+
Template template,
296+
Set<String> invisibleGetters,
297+
) : super(context, parent, template, invisibleGetters: invisibleGetters);
280298

281299
@override
282-
Property<Object> getProperty(String key) => null;
300+
Property<Object> getProperty(String key) {
301+
if (_invisibleGetters.contains(key)) {
302+
throw MustachioResolutionError(_failedKeyVisibilityMessage(key));
303+
} else {
304+
return null;
305+
}
306+
}
283307

284308
@override
285309
String getFields(Variable node) {
286310
var names = node.key;
287-
if (names.length == 1 && names.single == '.') {
311+
var firstName = node.key.first;
312+
if (names.length == 1 && firstName == '.') {
288313
return context.toString();
289-
}
290-
if (parent != null) {
314+
} else if (_invisibleGetters.contains(firstName)) {
315+
throw MustachioResolutionError(_failedKeyVisibilityMessage(firstName));
316+
} else if (parent != null) {
291317
return parent.getFields(node);
292318
} else {
293319
return 'null';
294320
}
295321
}
322+
323+
String _failedKeyVisibilityMessage(String name) {
324+
var type = context.runtimeType;
325+
return '[$name] is a getter on $type, which is not visible to Mustache. '
326+
'To render [$name] on $type, make it visible to Mustache via the '
327+
'`visibleTypes` parameter on `@Renderer`; to render [$name] on a '
328+
'different type up in the context stack, perhaps provide [$name] via '
329+
'a different name.';
330+
}
296331
}
297332

298333
/// An individual property of objects of type [T], including functions for
@@ -346,7 +381,7 @@ class Property<T> {
346381
class MustachioResolutionError extends Error {
347382
final String message;
348383

349-
MustachioResolutionError([this.message]);
384+
MustachioResolutionError(this.message);
350385

351386
@override
352387
String toString() => 'MustachioResolutionError: $message';
@@ -362,6 +397,17 @@ class PartialMustachioResolutionError extends Error {
362397
PartialMustachioResolutionError(this.name, this.contextType);
363398
}
364399

400+
/// A Mustachio resolution error which is thrown in a position where the AST
401+
/// node is not known.
402+
///
403+
/// This error should be caught and "re-thrown" as a [MustachioResolutionError]
404+
/// with a message derived from a [SourceSpan].
405+
class _MustachioResolutionErrorWithoutSpan extends Error {
406+
final String message;
407+
408+
_MustachioResolutionErrorWithoutSpan(this.message);
409+
}
410+
365411
extension MapExtensions<T> on Map<String, Property<T>> {
366412
Property<T> getValue(String name) {
367413
if (containsKey(name)) {

‎test/mustachio/foo.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class Foo extends FooBase<Baz> {
1717
@override
1818
Baz baz;
1919
Property1 p1;
20+
int length;
2021
}
2122

2223
class Bar {

‎test/mustachio/foo.runtime_renderers.dart‎

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ class Renderer_Bar extends RendererBase<Bar> {
8989
isNullValue: (CT_ c) => c.s2 == null,
9090
renderValue:
9191
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
92-
return renderSimple(c.s2, ast, r.template, parent: r);
92+
return renderSimple(c.s2, ast, r.template,
93+
parent: r, getters: _invisibleGetters['String']);
9394
},
9495
),
9596
});
@@ -210,8 +211,20 @@ class Renderer_Foo extends RendererBase<Foo> {
210211
self.renderSimpleVariable(c, remainingNames, 'List<int>'),
211212
renderIterable:
212213
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
213-
return c.l1.map(
214-
(e) => renderSimple(e, ast, r.template, parent: r));
214+
return c.l1.map((e) => renderSimple(e, ast, r.template,
215+
parent: r, getters: _invisibleGetters['int']));
216+
},
217+
),
218+
'length': Property(
219+
getValue: (CT_ c) => c.length,
220+
renderVariable: (CT_ c, Property<CT_> self,
221+
List<String> remainingNames) =>
222+
self.renderSimpleVariable(c, remainingNames, 'int'),
223+
isNullValue: (CT_ c) => c.length == null,
224+
renderValue:
225+
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
226+
return renderSimple(c.length, ast, r.template,
227+
parent: r, getters: _invisibleGetters['int']);
215228
},
216229
),
217230
'p1': Property(
@@ -241,7 +254,8 @@ class Renderer_Foo extends RendererBase<Foo> {
241254
isNullValue: (CT_ c) => c.s1 == null,
242255
renderValue:
243256
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
244-
return renderSimple(c.s1, ast, r.template, parent: r);
257+
return renderSimple(c.s1, ast, r.template,
258+
parent: r, getters: _invisibleGetters['String']);
245259
},
246260
),
247261
});
@@ -352,7 +366,8 @@ class Renderer_Object extends RendererBase<Object> {
352366
isNullValue: (CT_ c) => c.hashCode == null,
353367
renderValue:
354368
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
355-
return renderSimple(c.hashCode, ast, r.template, parent: r);
369+
return renderSimple(c.hashCode, ast, r.template,
370+
parent: r, getters: _invisibleGetters['int']);
356371
},
357372
),
358373
'runtimeType': Property(
@@ -364,7 +379,7 @@ class Renderer_Object extends RendererBase<Object> {
364379
renderValue:
365380
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
366381
return renderSimple(c.runtimeType, ast, r.template,
367-
parent: r);
382+
parent: r, getters: _invisibleGetters['Type']);
368383
},
369384
),
370385
});
@@ -457,7 +472,8 @@ class Renderer_Property2 extends RendererBase<Property2> {
457472
isNullValue: (CT_ c) => c.s == null,
458473
renderValue:
459474
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
460-
return renderSimple(c.s, ast, r.template, parent: r);
475+
return renderSimple(c.s, ast, r.template,
476+
parent: r, getters: _invisibleGetters['String']);
461477
},
462478
),
463479
});
@@ -499,7 +515,8 @@ class Renderer_Property3 extends RendererBase<Property3> {
499515
isNullValue: (CT_ c) => c.s == null,
500516
renderValue:
501517
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
502-
return renderSimple(c.s, ast, r.template, parent: r);
518+
return renderSimple(c.s, ast, r.template,
519+
parent: r, getters: _invisibleGetters['String']);
503520
},
504521
),
505522
});
@@ -517,3 +534,28 @@ class Renderer_Property3 extends RendererBase<Property3> {
517534
}
518535
}
519536
}
537+
538+
const _invisibleGetters = {
539+
'String': {
540+
'hashCode',
541+
'runtimeType',
542+
'length',
543+
'isEmpty',
544+
'isNotEmpty',
545+
'codeUnits',
546+
'runes'
547+
},
548+
'Type': {'hashCode', 'runtimeType'},
549+
'int': {
550+
'hashCode',
551+
'runtimeType',
552+
'isNaN',
553+
'isNegative',
554+
'isInfinite',
555+
'isFinite',
556+
'sign',
557+
'isEven',
558+
'isOdd',
559+
'bitLength'
560+
},
561+
};

‎test/mustachio/runtime_renderer_builder_test.dart‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ class Baz {}
131131
self.renderSimpleVariable(c, remainingNames, 'List<int>'),
132132
renderIterable:
133133
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
134-
return c.l1.map(
135-
(e) => renderSimple(e, ast, r.template, parent: r));
134+
return c.l1.map((e) => renderSimple(e, ast, r.template,
135+
parent: r, getters: _invisibleGetters['int']));
136136
},
137137
),
138138
'''));
@@ -148,7 +148,8 @@ class Baz {}
148148
isNullValue: (CT_ c) => c.s1 == null,
149149
renderValue:
150150
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
151-
return renderSimple(c.s1, ast, r.template, parent: r);
151+
return renderSimple(c.s1, ast, r.template,
152+
parent: r, getters: _invisibleGetters['String']);
152153
},
153154
),
154155
'''));

‎test/mustachio/runtime_renderer_render_test.dart‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@ void main() {
2525

2626
test('property map contains all public getters', () {
2727
var propertyMap = Renderer_Foo.propertyMap();
28-
expect(propertyMap.keys, hasLength(7));
28+
expect(propertyMap.keys, hasLength(8));
2929
expect(propertyMap['b1'], isNotNull);
3030
expect(propertyMap['s1'], isNotNull);
3131
expect(propertyMap['l1'], isNotNull);
3232
expect(propertyMap['baz'], isNotNull);
3333
expect(propertyMap['p1'], isNotNull);
34+
expect(propertyMap['length'], isNotNull);
3435
expect(propertyMap['hashCode'], isNotNull);
3536
expect(propertyMap['runtimeType'], isNotNull);
3637
});
@@ -516,6 +517,36 @@ line 1, column 9 of ${fooTemplateFile.path}: Failed to resolve 's2' as a propert
516517
contains('Failed to resolve [length] property chain on String'))));
517518
});
518519

520+
test('Renderer throws when a SimpleRenderer variable key shadows another key',
521+
() async {
522+
var fooTemplateFile = getFile('/project/foo.mustache')
523+
..writeAsStringSync('Text {{#s1}} {{length}} {{/s1}}');
524+
var fooTemplate = await Template.parse(fooTemplateFile);
525+
var foo = Foo()..s1 = 'String';
526+
expect(
527+
() => renderFoo(foo, fooTemplate),
528+
throwsA(const TypeMatcher<MustachioResolutionError>().having(
529+
(e) => e.message,
530+
'message',
531+
contains('[length] is a getter on String, which is not visible to '
532+
'Mustache.'))));
533+
});
534+
535+
test('Renderer throws when a SimpleRenderer section key shadows another key',
536+
() async {
537+
var fooTemplateFile = getFile('/project/foo.mustache')
538+
..writeAsStringSync('Text {{#s1}} {{#length}}Inner{{/length}} {{/s1}}');
539+
var fooTemplate = await Template.parse(fooTemplateFile);
540+
var foo = Foo()..s1 = 'String';
541+
expect(
542+
() => renderFoo(foo, fooTemplate),
543+
throwsA(const TypeMatcher<MustachioResolutionError>().having(
544+
(e) => e.message,
545+
'message',
546+
contains('[length] is a getter on String, which is not visible to '
547+
'Mustache.'))));
548+
});
549+
519550
test('Template parser throws when it cannot read a template', () async {
520551
var barTemplateFile = getFile('/project/src/bar.mustache');
521552
expect(

0 commit comments

Comments
 (0)