Skip to content

Commit e1c43f1

Browse files
kuhnroyalkuhnroyal
authored
[dart][dart-dio] Prevent name clashes with existing dart types (#8198)
* [dart][dart-dio] Prevent name clashes with existing dart types Can not use dart import aliases for now as this is not supported by built_value. This means we need to add potentially clashing names/classes to an `additionalReservedWords` exclusion list. Starting with a basic list of some http/io classes. Correctly use `importMapping` and `defaultIncludes` this time around. Improve reserved word checking. This now successfully generates `ModelList`, `ModelFile` and `ModelClient` models which previously were not generated at all or were wrong types. * Address review comment * Update generator docs
1 parent 80df0b0 commit e1c43f1

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+586
-405
lines changed

‎docs/generators/dart-dio.md

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,8 @@ These options may be applied as additional-properties (cli) or configOptions (pl
3333
|BuiltList|package:built_collection/built_collection.dart|
3434
|BuiltMap|package:built_collection/built_collection.dart|
3535
|BuiltSet|package:built_collection/built_collection.dart|
36-
|DateTime|dart:core|
3736
|JsonObject|package:built_value/json_object.dart|
38-
|List|dart:core|
39-
|Map|dart:core|
40-
|Object|dart:core|
41-
|Set|dart:core|
42-
|String|dart:core|
4337
|Uint8List|dart:typed_data|
44-
|bool|dart:core|
45-
|double|dart:core|
46-
|dynamic|dart:core|
47-
|int|dart:core|
48-
|num|dart:core|
4938

5039

5140
## INSTANTIATION TYPES
@@ -62,6 +51,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
6251
<li>String</li>
6352
<li>bool</li>
6453
<li>double</li>
54+
<li>dynamic</li>
6555
<li>int</li>
6656
<li>num</li>
6757
</ul>

‎docs/generators/dart-jaguar.md

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,6 @@ These options may be applied as additional-properties (cli) or configOptions (pl
3030

3131
| Type/Alias | Imports |
3232
| ---------- | ------- |
33-
|DateTime|dart:core|
34-
|List|dart:core|
35-
|Map|dart:core|
36-
|Object|dart:core|
37-
|Set|dart:core|
38-
|String|dart:core|
39-
|bool|dart:core|
40-
|double|dart:core|
41-
|dynamic|dart:core|
42-
|int|dart:core|
43-
|num|dart:core|
4433

4534

4635
## INSTANTIATION TYPES
@@ -57,6 +46,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
5746
<li>String</li>
5847
<li>bool</li>
5948
<li>double</li>
49+
<li>dynamic</li>
6050
<li>int</li>
6151
<li>num</li>
6252
</ul>

‎docs/generators/dart.md

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,6 @@ These options may be applied as additional-properties (cli) or configOptions (pl
2828

2929
| Type/Alias | Imports |
3030
| ---------- | ------- |
31-
|DateTime|dart:core|
32-
|List|dart:core|
33-
|Map|dart:core|
34-
|Object|dart:core|
35-
|Set|dart:core|
36-
|String|dart:core|
37-
|bool|dart:core|
38-
|double|dart:core|
39-
|dynamic|dart:core|
40-
|int|dart:core|
41-
|num|dart:core|
4231

4332

4433
## INSTANTIATION TYPES
@@ -55,6 +44,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
5544
<li>String</li>
5645
<li>bool</li>
5746
<li>double</li>
47+
<li>dynamic</li>
5848
<li>int</li>
5949
<li>num</li>
6050
</ul>

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/DartClientCodegen.java

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ public class DartClientCodegen extends DefaultCodegen {
7373
protected String apiTestPath = "test" + File.separator;
7474
protected String modelTestPath = "test" + File.separator;
7575

76+
// Names that must not be used as model names because they clash with existing
77+
// default imports (dart:io, dart:async, package:http etc.) but are not basic dataTypes.
78+
protected Set<String> additionalReservedWords;
79+
7680
public DartClientCodegen() {
7781
super();
7882

@@ -131,7 +135,8 @@ public DartClientCodegen() {
131135
"bool",
132136
"int",
133137
"num",
134-
"double"
138+
"double",
139+
"dynamic"
135140
);
136141
instantiationTypes.put("array", "List");
137142
instantiationTypes.put("map", "Map");
@@ -155,29 +160,37 @@ public DartClientCodegen() {
155160
typeMapping.put("Date", "DateTime");
156161
typeMapping.put("date", "DateTime");
157162
typeMapping.put("DateTime", "DateTime");
158-
typeMapping.put("File", "MultipartFile");
163+
typeMapping.put("file", "MultipartFile");
159164
typeMapping.put("binary", "MultipartFile");
160165
typeMapping.put("UUID", "String");
161166
typeMapping.put("URI", "String");
162167
typeMapping.put("ByteArray", "String");
163168
typeMapping.put("object", "Object");
164169
typeMapping.put("AnyType", "Object");
165170

166-
// These are needed as they prevent models from being generated
167-
// which would clash with existing types, e.g. List
168-
importMapping.put("dynamic", "dart:core");
169-
importMapping.put("Object", "dart:core");
170-
importMapping.put("String", "dart:core");
171-
importMapping.put("bool", "dart:core");
172-
importMapping.put("num", "dart:core");
173-
importMapping.put("int", "dart:core");
174-
importMapping.put("double", "dart:core");
175-
importMapping.put("List", "dart:core");
176-
importMapping.put("Map", "dart:core");
177-
importMapping.put("Set", "dart:core");
178-
importMapping.put("DateTime", "dart:core");
179-
180-
defaultIncludes = new HashSet<>(Collections.singletonList("dart:core"));
171+
// DataTypes of the above values which are automatically imported.
172+
// They are also not allowed to be model names.
173+
defaultIncludes = Sets.newHashSet(
174+
"String",
175+
"bool",
176+
"int",
177+
"num",
178+
"double",
179+
"dynamic",
180+
"List",
181+
"Set",
182+
"Map",
183+
"DateTime",
184+
"Object",
185+
"MultipartFile"
186+
);
187+
188+
additionalReservedWords = Sets.newHashSet(
189+
"File",
190+
"Client",
191+
"Future",
192+
"Response"
193+
);
181194

182195
cliOptions.add(new CliOption(PUB_LIBRARY, "Library name in generated code"));
183196
cliOptions.add(new CliOption(PUB_NAME, "Name in generated pubspec"));
@@ -306,6 +319,15 @@ public void processOpts() {
306319
supportingFiles.add(new SupportingFile("travis.mustache", "", ".travis.yml"));
307320
}
308321

322+
@Override
323+
protected boolean isReservedWord(String word) {
324+
// consider everything as reserved that is either a keyword,
325+
// a default included type, or a type include through some library
326+
return super.isReservedWord(word) ||
327+
defaultIncludes().contains(word) ||
328+
additionalReservedWords.contains(word);
329+
}
330+
309331
@Override
310332
public String escapeReservedWord(String name) {
311333
return name + "_";
@@ -370,7 +392,7 @@ public String toVarName(String name) {
370392
name = "n" + name;
371393
}
372394

373-
if (isReservedWord(name) || importMapping().containsKey(name)) {
395+
if (isReservedWord(name)) {
374396
name = escapeReservedWord(name);
375397
}
376398

@@ -385,10 +407,6 @@ public String toParamName(String name) {
385407

386408
@Override
387409
public String toModelName(final String name) {
388-
if (importMapping().containsKey(name)) {
389-
return name;
390-
}
391-
392410
String nameWithPrefixSuffix = sanitizeName(name);
393411
if (!StringUtils.isEmpty(modelNamePrefix)) {
394412
// add '_' so that model name can be camelized correctly

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/DartDioClientCodegen.java

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,9 @@
1919
import com.google.common.collect.ImmutableMap;
2020
import com.google.common.collect.Sets;
2121
import com.samskivert.mustache.Mustache;
22+
import io.swagger.v3.oas.models.media.Schema;
2223
import org.apache.commons.lang3.StringUtils;
23-
import org.openapitools.codegen.CliOption;
24-
import org.openapitools.codegen.CodegenConstants;
25-
import org.openapitools.codegen.CodegenModel;
26-
import org.openapitools.codegen.CodegenOperation;
27-
import org.openapitools.codegen.CodegenParameter;
28-
import org.openapitools.codegen.CodegenProperty;
29-
import org.openapitools.codegen.SupportingFile;
24+
import org.openapitools.codegen.*;
3025
import org.openapitools.codegen.utils.ModelUtils;
3126
import org.openapitools.codegen.utils.ProcessUtils;
3227
import org.slf4j.Logger;
@@ -35,8 +30,6 @@
3530
import java.io.File;
3631
import java.util.*;
3732

38-
import io.swagger.v3.oas.models.media.Schema;
39-
4033
import static org.openapitools.codegen.utils.StringUtils.underscore;
4134

4235
public class DartDioClientCodegen extends DartClientCodegen {
@@ -49,7 +42,6 @@ public class DartDioClientCodegen extends DartClientCodegen {
4942

5043
private boolean nullableFields = true;
5144
private String dateLibrary = "core";
52-
private static final Set<String> reservedBuiltValueWords = Sets.newHashSet("EnumClass");
5345

5446
public DartDioClientCodegen() {
5547
super();
@@ -75,6 +67,17 @@ public DartDioClientCodegen() {
7567
typeMapping.put("object", "JsonObject");
7668
typeMapping.put("AnyType", "JsonObject");
7769

70+
additionalReservedWords.addAll(Sets.newHashSet(
71+
"EnumClass",
72+
// The following are reserved dataTypes but can not be added to defaultIncludes
73+
// as this would prevent them from being added to the imports.
74+
"BuiltList",
75+
"BuiltSet",
76+
"BuiltMap",
77+
"Uint8List",
78+
"JsonObject"
79+
));
80+
7881
importMapping.put("BuiltList", "package:built_collection/built_collection.dart");
7982
importMapping.put("BuiltSet", "package:built_collection/built_collection.dart");
8083
importMapping.put("BuiltMap", "package:built_collection/built_collection.dart");
@@ -108,11 +111,6 @@ public String getHelp() {
108111
return "Generates a Dart Dio client library.";
109112
}
110113

111-
@Override
112-
protected boolean isReservedWord(String word) {
113-
return super.isReservedWord(word) || reservedBuiltValueWords.contains(word);
114-
}
115-
116114
@Override
117115
protected ImmutableMap.Builder<String, Mustache.Lambda> addMustacheLambdas() {
118116
return super.addMustacheLambdas()
@@ -227,19 +225,18 @@ public void processOpts() {
227225
supportingFiles.add(new SupportingFile("auth/auth.mustache", authFolder, "auth.dart"));
228226

229227
if ("core".equals(dateLibrary)) {
228+
// this option uses the same classes as normal dart generator
230229
additionalProperties.put("core", "true");
231-
typeMapping.put("Date", "DateTime");
232-
typeMapping.put("date", "DateTime");
233230
} else if ("timemachine".equals(dateLibrary)) {
234231
additionalProperties.put("timeMachine", "true");
235232
typeMapping.put("date", "OffsetDate");
236233
typeMapping.put("Date", "OffsetDate");
237234
typeMapping.put("DateTime", "OffsetDateTime");
238235
typeMapping.put("datetime", "OffsetDateTime");
236+
additionalReservedWords.addAll(Sets.newHashSet("OffsetDate", "OffsetDateTime"));
239237
importMapping.put("OffsetDate", "package:time_machine/time_machine.dart");
240238
importMapping.put("OffsetDateTime", "package:time_machine/time_machine.dart");
241239
supportingFiles.add(new SupportingFile("local_date_serializer.mustache", libFolder, "local_date_serializer.dart"));
242-
243240
}
244241
}
245242

@@ -254,13 +251,12 @@ public Map<String, Object> postProcessModels(Map<String, Object> objs) {
254251
Set<String> modelImports = new HashSet<>();
255252
CodegenModel cm = (CodegenModel) mo.get("model");
256253
for (String modelImport : cm.imports) {
257-
if (importMapping().containsKey(modelImport)) {
258-
final String value = importMapping().get(modelImport);
259-
if (needToImport(value)) {
260-
modelImports.add(value);
254+
if (needToImport(modelImport)) {
255+
if (importMapping().containsKey(modelImport)) {
256+
modelImports.add(importMapping().get(modelImport));
257+
} else {
258+
modelImports.add("package:" + pubName + "/model/" + underscore(modelImport) + ".dart");
261259
}
262-
} else {
263-
modelImports.add("package:" + pubName + "/model/" + underscore(modelImport) + ".dart");
264260
}
265261
}
266262

@@ -327,18 +323,16 @@ public Map<String, Object> postProcessOperationsWithModels(Map<String, Object> o
327323

328324
Set<String> imports = new HashSet<>();
329325
for (String item : op.imports) {
330-
if (importMapping().containsKey(item)) {
331-
final String value = importMapping().get(item);
332-
if (needToImport(value)) {
333-
fullImports.add(value);
326+
if (needToImport(item)) {
327+
if (importMapping().containsKey(item) && needToImport(item)) {
328+
fullImports.add(importMapping().get(item));
329+
} else {
330+
imports.add(underscore(item));
334331
}
335-
} else {
336-
imports.add(underscore(item));
337332
}
338333
}
339334
modelImports.addAll(imports);
340335
op.imports = imports;
341-
342336
}
343337

344338
objs.put("modelImports", modelImports);

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/dart/DartModelTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,10 +295,15 @@ public static Object[][] modelNames() {
295295
{"sample.name", "SampleName"},
296296
{"_sample", "Sample"},
297297
{"sample name", "SampleName"},
298+
{"List", "ModelList"},
299+
{"list", "ModelList"},
300+
{"File", "ModelFile"},
301+
{"Client", "ModelClient"},
302+
{"String", "ModelString"},
298303
};
299304
}
300305

301-
@Test(dataProvider = "modelNames", description = "avoid inner class")
306+
@Test(dataProvider = "modelNames", description = "correctly generate model names")
302307
public void modelNameTest(String name, String expectedName) {
303308
OpenAPI openAPI = TestUtils.createOpenAPI();
304309
final Schema model = new Schema();

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/dartdio/DartDioModelTest.java

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,14 +350,41 @@ public void mapModelTest() {
350350
public static Object[][] modelNames() {
351351
return new Object[][] {
352352
{"EnumClass", "ModelEnumClass"},
353+
{"JsonObject", "ModelJsonObject"},
354+
// OffsetDate is valid without timemachine date library
355+
{"OffsetDate", "OffsetDate"},
353356
};
354357
}
355358

356-
@Test(dataProvider = "modelNames", description = "avoid inner class")
359+
@Test(dataProvider = "modelNames", description = "correctly prefix reserved model names")
357360
public void modelNameTest(String name, String expectedName) {
358361
OpenAPI openAPI = TestUtils.createOpenAPI();
359362
final Schema model = new Schema();
360-
final DefaultCodegen codegen = new DartDioClientCodegen();
363+
final DartDioClientCodegen codegen = new DartDioClientCodegen();
364+
codegen.setOpenAPI(openAPI);
365+
final CodegenModel cm = codegen.fromModel(name, model);
366+
367+
Assert.assertEquals(cm.name, name);
368+
Assert.assertEquals(cm.classname, expectedName);
369+
}
370+
371+
@DataProvider(name = "modelNamesTimemachine")
372+
public static Object[][] modelNamesTimemachine() {
373+
return new Object[][] {
374+
{"EnumClass", "ModelEnumClass"},
375+
{"JsonObject", "ModelJsonObject"},
376+
// OffsetDate is not valid with timemachine date library
377+
{"OffsetDate", "ModelOffsetDate"},
378+
};
379+
}
380+
381+
@Test(dataProvider = "modelNamesTimemachine", description = "correctly prefix reserved model names")
382+
public void modelNameTestTimemachine(String name, String expectedName) {
383+
OpenAPI openAPI = TestUtils.createOpenAPI();
384+
final Schema model = new Schema();
385+
final DartDioClientCodegen codegen = new DartDioClientCodegen();
386+
codegen.setDateLibrary("timemachine");
387+
codegen.processOpts();
361388
codegen.setOpenAPI(openAPI);
362389
final CodegenModel cm = codegen.fromModel(name, model);
363390

0 commit comments

Comments
 (0)