Skip to content

Commit 8cf5e87

Browse files
authored
Fix three little issues in mustachio codegen (#2491)
Fix three little issues in mustachio codegen: * Allow duplicate types in different `@Renderer` specifications. * Only build each render class and renderer method once; you cannot declare two classes or functions with the same name. * Do not attempt to add generators for type parameters; instead, use the bound if it is not `dynamic`.
1 parent 3309e49 commit 8cf5e87

File tree

2 files changed

+61
-20
lines changed

2 files changed

+61
-20
lines changed

test/mustachio/builder_test.dart

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class Context<T> {
2525
};
2626

2727
const _libraryFrontMatter = '''
28-
@Renderer(#renderFoo, Context<Foo>(), visibleTypes: {Bar})
28+
@Renderer(#renderFoo, Context<Foo>(), visibleTypes: {Bar, Baz})
2929
library foo;
3030
import 'package:mustachio/annotations.dart';
3131
''';
@@ -73,15 +73,19 @@ $sourceLibraryContent
7373
setUpAll(() async {
7474
writer = InMemoryAssetWriter();
7575
await testMustachioBuilder('''
76-
abstract class FooBase {
76+
abstract class FooBase2<T> {
77+
T get generic;
78+
}
79+
abstract class FooBase<T extends Baz> extends FooBase2<T> {
7780
Bar get bar;
7881
}
79-
class Foo extends FooBase {
82+
abstract class Foo extends FooBase {
8083
String s1 = "s1";
8184
bool b1 = false;
8285
List<int> l1 = [1, 2, 3];
8386
}
8487
class Bar {}
88+
class Baz {}
8589
''');
8690
renderersLibrary = await resolveGeneratedLibrary(writer);
8791
var rendererAsset = AssetId.parse('foo|lib/foo.renderers.dart');
@@ -92,11 +96,13 @@ class Bar {}
9296
// The render function for Foo
9397
expect(
9498
generatedContent,
95-
contains('String _render_FooBase(\n'
96-
' FooBase context, List<MustachioNode> ast, Template template,'));
99+
contains('String _render_FooBase<T extends Baz>(\n'
100+
' FooBase<T> context, List<MustachioNode> ast, Template template,'));
97101
// The renderer class for Foo
98-
expect(generatedContent,
99-
contains('class _Renderer_FooBase extends RendererBase<FooBase>'));
102+
expect(
103+
generatedContent,
104+
contains(
105+
'class _Renderer_FooBase<T extends Baz> extends RendererBase<FooBase<T>>'));
100106
});
101107

102108
test('for Object', () {
@@ -121,6 +127,11 @@ class Bar {}
121127
expect(renderersLibrary.getType('_Renderer_Bar'), isNotNull);
122128
});
123129

130+
test('for a generic, bounded type found in a getter', () {
131+
expect(renderersLibrary.getTopLevelFunction('_render_Baz'), isNotNull);
132+
expect(renderersLibrary.getType('_Renderer_Baz'), isNotNull);
133+
});
134+
124135
test('with a property map', () {
125136
expect(
126137
generatedContent,
@@ -130,7 +141,7 @@ class Bar {}
130141

131142
test('with a property map which references the superclass', () {
132143
expect(generatedContent,
133-
contains('..._Renderer_FooBase.propertyMap<CT_>(),'));
144+
contains('..._Renderer_FooBase.propertyMap<Baz, CT_>(),'));
134145
});
135146

136147
test('with a property map with a bool property', () {
@@ -203,6 +214,7 @@ class Bar {}
203214
await testMustachioBuilder('''
204215
class Foo {}
205216
class Bar {}
217+
class Baz {}
206218
''', libraryFrontMatter: '''
207219
@Renderer(#renderFoo, Context<Foo>())
208220
@Renderer(#renderBar, Context<Bar>())
@@ -229,6 +241,7 @@ class FooBase<T> {}
229241
class Foo<T> extends FooBase<T> {}
230242
class BarBase<T> {}
231243
class Bar<T> extends BarBase<int> {}
244+
class Baz {}
232245
''', libraryFrontMatter: '''
233246
@Renderer(#renderFoo, Context<Foo>())
234247
@Renderer(#renderBar, Context<Bar>())
@@ -275,6 +288,7 @@ import 'package:mustachio/annotations.dart';
275288
await testMustachioBuilder('''
276289
class Foo<T extends num> {}
277290
class Bar {}
291+
class Baz {}
278292
''');
279293
var renderersLibrary = await resolveGeneratedLibrary(writer);
280294

@@ -295,13 +309,14 @@ class Bar {}
295309
setUpAll(() async {
296310
writer = InMemoryAssetWriter();
297311
await testMustachioBuilder('''
298-
class Foo {
312+
abstract class Foo<T> {
299313
static Static get static1 => Bar();
300314
Private get _private1 => Bar();
301315
void set setter1(Setter s);
302316
Method method1(Method m);
303317
}
304318
class Bar {}
319+
class Baz {}
305320
class Static {}
306321
class Private {}
307322
class Setter {}

tool/mustachio/codegen_runtime_renderer.dart

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,16 @@ String _simpleResolveErrorMessage(List<String> key, String type) =>
101101

102102
specs.forEach(_addTypesForRendererSpec);
103103

104+
var builtRenderers = <ClassElement>{};
105+
104106
while (_typesToProcess.isNotEmpty) {
105107
var info = _typesToProcess.removeFirst();
106108

107109
if (info.isFullRenderer) {
108-
_buildRenderer(info);
110+
var buildOnlyPublicFunction =
111+
builtRenderers.contains(info._contextClass);
112+
_buildRenderer(info, buildOnlyPublicFunction: buildOnlyPublicFunction);
113+
builtRenderers.add(info._contextClass);
109114
}
110115
}
111116

@@ -142,6 +147,18 @@ String _simpleResolveErrorMessage(List<String> key, String type) =>
142147
if (property.isPrivate || property.isStatic || property.isSetter) return;
143148
if (property.hasProtected || property.hasVisibleForTesting) return;
144149
var type = property.type.returnType;
150+
if (type is TypeParameterType) {
151+
var bound = (type as TypeParameterType).bound;
152+
if (bound == null || bound.isDynamic) {
153+
// Don't add functions for a generic type, for example
154+
// `List<E>.first` has type `E`, which we don't have a specific
155+
// renderer for.
156+
// TODO(srawlins): Find a solution for this. We can track all of the
157+
// concrete types substituted for `E` for example.
158+
return;
159+
}
160+
type = bound;
161+
}
145162
var isFullRenderer = _isVisibleToMustache(type.element);
146163

147164
if (_typeSystem.isAssignableTo(type, _typeProvider.iterableDynamicType)) {
@@ -170,9 +187,8 @@ String _simpleResolveErrorMessage(List<String> key, String type) =>
170187

171188
/// Adds [type] to the [_typesToProcess] queue, if it is not already there.
172189
void _addTypeToProcess(ClassElement element, {@required isFullRenderer}) {
173-
var typeToProcess = _typesToProcess
174-
.singleWhere((rs) => rs._contextClass == element, orElse: () => null);
175-
if (typeToProcess == null) {
190+
var types = _typesToProcess.where((rs) => rs._contextClass == element);
191+
if (types.isEmpty) {
176192
var rendererInfo = _RendererInfo(element,
177193
isFullRenderer: isFullRenderer, public: _rendererClassesArePublic);
178194
_typesToProcess.add(rendererInfo);
@@ -181,11 +197,14 @@ String _simpleResolveErrorMessage(List<String> key, String type) =>
181197
_typeToRendererClassName[element] = rendererInfo._rendererClassName;
182198
}
183199
} else {
184-
if (isFullRenderer && !typeToProcess.isFullRenderer) {
185-
// This is the only case in which we update a type-to-render.
186-
typeToProcess.isFullRenderer = true;
187-
_typeToRenderFunctionName[element] = typeToProcess._renderFunctionName;
188-
_typeToRendererClassName[element] = typeToProcess._rendererClassName;
200+
for (var typeToProcess in types) {
201+
if (isFullRenderer && !typeToProcess.isFullRenderer) {
202+
// This is the only case in which we update a type-to-render.
203+
typeToProcess.isFullRenderer = true;
204+
_typeToRenderFunctionName[element] =
205+
typeToProcess._renderFunctionName;
206+
_typeToRendererClassName[element] = typeToProcess._rendererClassName;
207+
}
189208
}
190209
}
191210
}
@@ -201,14 +220,19 @@ String _simpleResolveErrorMessage(List<String> key, String type) =>
201220
return _isVisibleToMustache(element.supertype.element);
202221
}
203222

204-
/// Builds both the render function and the renderer class for [renderer].
223+
/// Builds render functions and the renderer class for [renderer].
205224
///
206225
/// The function and the class are each written as Dart code to [_buffer].
207226
///
208227
/// If [renderer] also specifies a `publicApiFunctionName`, then a public API
209228
/// function (which renders a context object using a template file at a path,
210229
/// rather than an AST) is also written.
211-
void _buildRenderer(_RendererInfo renderer) {
230+
///
231+
/// If [buildOnlyPublicFunction] is true, then the private render function and
232+
/// renderer classes are not built, having been built for a different
233+
/// [_RendererInfo].
234+
void _buildRenderer(_RendererInfo renderer,
235+
{@required bool buildOnlyPublicFunction}) {
212236
var typeName = renderer._typeName;
213237
var typeWithVariables = '$typeName${renderer._typeVariablesString}';
214238

@@ -221,6 +245,8 @@ String ${renderer.publicApiFunctionName}${renderer._typeParametersString}(
221245
''');
222246
}
223247

248+
if (buildOnlyPublicFunction) return;
249+
224250
// Write out the render function.
225251
_buffer.writeln('''
226252
String ${renderer._renderFunctionName}${renderer._typeParametersString}(

0 commit comments

Comments
 (0)