Skip to content

Commit

Permalink
[dart] Variable/Operation/Model naming (#8123)
Browse files Browse the repository at this point in the history 
* [dart] Properly escape param/var names that clash with Dart types

* [dart] Add tests for var/enumVar names

* note: public and private are no keywords in Dart
* some tests are still wrong, some are commented out

* Fix typo

* [dart] Improve variable name escaping

* add more tests

* [dart] Fix operationId naming

* [dart] Fix upper case var name with leading underscore

* [dart] Correctly support model prefix/suffix

* [dart] Support spaces in property names

* Call super for empty operationId

* [dart] Fix and improve enumVar naming/generation

* use same handling for all 3 generators
* allow `updateEnumVarsWithExtensions` to have access to the data type
* improve `x-enum-values` handling and add supprt for other enum vendor extensions
* remove duplicate and outdated tests from `DartDioModelTest`
* add more tests to `DartModelTest`
* no longer force lowercase enums for plain dart generator (breaking)
* this change also removes the trailing underscore from plain dart generator (breaking)
  • Loading branch information
@kuhnroyal
kuhnroyal authored Dec 9, 2020
1 parent 828e924 commit 4ad6d0b
Show file tree
Hide file tree
Showing 67 changed files with 621 additions and 534 deletions.
14 changes: 7 additions & 7 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ public Map<String, Object> postProcessModelsEnum(Map<String, Object> objs) {
List<Object> values = (List<Object>) allowableValues.get("values");
List<Map<String, Object>> enumVars = buildEnumVars(values, cm.dataType);
// if "x-enum-varnames" or "x-enum-descriptions" defined, update varnames
updateEnumVarsWithExtensions(enumVars, cm.getVendorExtensions());
updateEnumVarsWithExtensions(enumVars, cm.getVendorExtensions(), cm.dataType);
cm.allowableValues.put("enumVars", enumVars);
}

Expand Down Expand Up @@ -1313,7 +1313,7 @@ public String toOperationId(String operationId) {
public String toVarName(final String name) {
if (reservedWords.contains(name)) {
return escapeReservedWord(name);
} else if (((CharSequence) name).chars().anyMatch(character -> specialCharReplacements.keySet().contains("" + ((char) character)))) {
} else if (name.chars().anyMatch(character -> specialCharReplacements.containsKey("" + ((char) character)))) {
return escape(name, specialCharReplacements, null, null);
}
return name;
Expand All @@ -1330,7 +1330,7 @@ public String toParamName(String name) {
name = removeNonNameElementToCamelCase(name); // FIXME: a parameter should not be assigned. Also declare the methods parameters as 'final'.
if (reservedWords.contains(name)) {
return escapeReservedWord(name);
} else if (((CharSequence) name).chars().anyMatch(character -> specialCharReplacements.keySet().contains("" + ((char) character)))) {
} else if (name.chars().anyMatch(character -> specialCharReplacements.containsKey("" + ((char) character)))) {
return escape(name, specialCharReplacements, null, null);
}
return name;
Expand Down Expand Up @@ -1506,7 +1506,7 @@ public DefaultCodegen() {
this.setDisallowAdditionalPropertiesIfNotPresent(true);

// initialize special character mapping
initalizeSpecialCharacterMapping();
initializeSpecialCharacterMapping();

// Register common Mustache lambdas.
registerMustacheLambdas();
Expand All @@ -1515,7 +1515,7 @@ public DefaultCodegen() {
/**
* Initialize special character mapping
*/
protected void initalizeSpecialCharacterMapping() {
protected void initializeSpecialCharacterMapping() {
// Initialize special characters
specialCharReplacements.put("$", "Dollar");
specialCharReplacements.put("^", "Caret");
Expand Down Expand Up @@ -5441,7 +5441,7 @@ public void updateCodegenPropertyEnum(CodegenProperty var) {
if (referencedSchema.isPresent()) {
extensions = referencedSchema.get().getExtensions();
}
updateEnumVarsWithExtensions(enumVars, extensions);
updateEnumVarsWithExtensions(enumVars, extensions, dataType);
allowableValues.put("enumVars", enumVars);

// handle default value for enum, e.g. available => StatusEnum.AVAILABLE
Expand Down Expand Up @@ -5494,7 +5494,7 @@ protected List<Map<String, Object>> buildEnumVars(List<Object> values, String da
return enumVars;
}

protected void updateEnumVarsWithExtensions(List<Map<String, Object>> enumVars, Map<String, Object> vendorExtensions) {
protected void updateEnumVarsWithExtensions(List<Map<String, Object>> enumVars, Map<String, Object> vendorExtensions, String dataType) {
if (vendorExtensions != null) {
updateEnumVarsWithExtensions(enumVars, vendorExtensions, "x-enum-varnames", "name");
updateEnumVarsWithExtensions(enumVars, vendorExtensions, "x-enum-descriptions", "enumDescription");
Expand Down
2 changes: 1 addition & 1 deletion ...enapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,7 @@ public String toOperationId(String operationId) {

// operationId starts with a number
if (operationId.matches("^\\d.*")) {
LOGGER.warn(operationId + " (starting with a number) cannot be used as method sname. Renamed to " + camelize("call_" + operationId), true);
LOGGER.warn(operationId + " (starting with a number) cannot be used as method name. Renamed to " + camelize("call_" + operationId), true);
operationId = camelize("call_" + operationId, true);
}

Expand Down
193 changes: 94 additions & 99 deletions ...openapi-generator/src/main/java/org/openapitools/codegen/languages/DartClientCodegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@

package org.openapitools.codegen.languages;

import static org.openapitools.codegen.utils.StringUtils.camelize;
import static org.openapitools.codegen.utils.StringUtils.underscore;

import java.io.BufferedReader;
import java.io.File;
import java.io.InputStreamReader;
Expand All @@ -31,14 +28,13 @@
import java.util.Map;
import java.util.Set;

import com.google.common.collect.Lists;
import com.google.common.collect.Sets;

import org.apache.commons.io.FilenameUtils;
import org.apache.commons.lang3.StringUtils;
import org.openapitools.codegen.CliOption;
import org.openapitools.codegen.CodegenConstants;
import org.openapitools.codegen.CodegenModel;
import org.openapitools.codegen.CodegenProperty;
import org.openapitools.codegen.CodegenType;
import org.openapitools.codegen.DefaultCodegen;
import org.openapitools.codegen.SupportingFile;
Expand All @@ -55,6 +51,8 @@
import io.swagger.v3.oas.models.media.ArraySchema;
import io.swagger.v3.oas.models.media.Schema;

import static org.openapitools.codegen.utils.StringUtils.*;

public class DartClientCodegen extends DefaultCodegen {
private static final Logger LOGGER = LoggerFactory.getLogger(DartClientCodegen.class);

Expand Down Expand Up @@ -150,6 +148,7 @@ public DartClientCodegen() {
typeMapping = new HashMap<>();
typeMapping.put("Array", "List");
typeMapping.put("array", "List");
typeMapping.put("map", "Map");
typeMapping.put("List", "List");
typeMapping.put("boolean", "bool");
typeMapping.put("string", "String");
Expand All @@ -164,6 +163,7 @@ public DartClientCodegen() {
typeMapping.put("integer", "int");
typeMapping.put("Date", "DateTime");
typeMapping.put("date", "DateTime");
typeMapping.put("DateTime", "DateTime");
typeMapping.put("File", "MultipartFile");
typeMapping.put("binary", "MultipartFile");
typeMapping.put("UUID", "String");
Expand Down Expand Up @@ -352,23 +352,38 @@ public String modelDocFileFolder() {

@Override
public String toVarName(String name) {
return toVarName(name, "n");
}

private String toVarName(String name, String numberPrefix) {
// replace - with _ e.g. created-at => created_at
name = name.replaceAll("-", "_");
name = name.replace("-", "_");

// always need to replace leading underscores first
name = name.replaceAll("^_", "");

// if it's all upper case, do nothing
if (name.matches("^[A-Z_]*$")) {
return name;
}

// replace all characters that have a mapping but ignore underscores
// append an underscore to each replacement so that it can be camelized
if (name.chars().anyMatch(character -> specialCharReplacements.containsKey("" + ((char) character)))) {
name = escape(name, specialCharReplacements, Lists.newArrayList("_"), "_");
}
// remove the rest
name = sanitizeName(name);

// camelize (lower first character) the variable name
// pet_id => petId
name = camelize(name, true);

if (name.matches("^\\d.*")) {
name = "n" + name;
name = numberPrefix + name;
}

if (isReservedWord(name)) {
if (isReservedWord(name) || importMapping().containsKey(name)) {
name = escapeReservedWord(name);
}

Expand All @@ -382,24 +397,41 @@ public String toParamName(String name) {
}

@Override
public String toModelName(String name) {
// model name cannot use reserved keyword, e.g. return
if (isReservedWord(name)) {
LOGGER.warn(name + " (reserved word) cannot be used as model filename. Renamed to " + camelize("model_" + name));
name = "model_" + name; // e.g. return => ModelReturn (after camelize)
public String toModelName(final String name) {
if (importMapping().containsKey(name)) {
return name;
}

if (name.matches("^\\d.*")) {
LOGGER.warn(name + " (model name starts with number) cannot be used as model name. Renamed to " + camelize("model_" + name));
name = "model_" + name; // e.g. 200Response => Model200Response (after camelize)
String nameWithPrefixSuffix = sanitizeName(name);
if (!StringUtils.isEmpty(modelNamePrefix)) {
// add '_' so that model name can be camelized correctly
nameWithPrefixSuffix = modelNamePrefix + "_" + nameWithPrefixSuffix;
}

if (typeMapping.containsValue(name)) {
return camelize(name);
} else {
// camelize the model name
return camelize(modelNamePrefix + "_" + name + "_" + modelNameSuffix);
if (!StringUtils.isEmpty(modelNameSuffix)) {
// add '_' so that model name can be camelized correctly
nameWithPrefixSuffix = nameWithPrefixSuffix + "_" + modelNameSuffix;
}

// camelize the model name
// phone_number => PhoneNumber
final String camelizedName = camelize(nameWithPrefixSuffix);

// model name cannot use reserved keyword, e.g. return
if (isReservedWord(camelizedName)) {
final String modelName = "Model" + camelizedName;
LOGGER.warn(camelizedName + " (reserved word) cannot be used as model name. Renamed to " + modelName);
return modelName;
}

// model name starts with number
if (camelizedName.matches("^\\d.*")) {
final String modelName = "Model" + camelizedName; // e.g. 200Response => Model200Response (after camelize)
LOGGER.warn(name + " (model name starts with number) cannot be used as model name. Renamed to " + modelName);
return modelName;
}

return camelizedName;
}

@Override
Expand All @@ -408,7 +440,7 @@ public String toModelFilename(String name) {
}

@Override public String toModelDocFilename(String name) {
return super.toModelDocFilename(toModelName(name));
return toModelName(name);
}

@Override
Expand Down Expand Up @@ -436,7 +468,7 @@ public String toDefaultValue(Schema schema) {

if (schema.getDefault() != null) {
if (ModelUtils.isStringSchema(schema)) {
return "'" + schema.getDefault().toString().replaceAll("'", "\\'") + "'";
return "'" + schema.getDefault().toString().replace("'", "\\'") + "'";
}
return schema.getDefault().toString();
} else {
Expand All @@ -449,26 +481,22 @@ public String getTypeDeclaration(Schema p) {
if (ModelUtils.isArraySchema(p)) {
ArraySchema ap = (ArraySchema) p;
Schema inner = ap.getItems();
return getSchemaType(p) + "<" + getTypeDeclaration(inner) + ">";
return instantiationTypes().get("array") + "<" + getTypeDeclaration(inner) + ">";
} else if (ModelUtils.isMapSchema(p)) {
Schema inner = getAdditionalProperties(p);

return getSchemaType(p) + "<String, " + getTypeDeclaration(inner) + ">";
return instantiationTypes().get("map") + "<String, " + getTypeDeclaration(inner) + ">";
}
return super.getTypeDeclaration(p);
}

@Override
public String getSchemaType(Schema p) {
String openAPIType = super.getSchemaType(p);
String type;
if (typeMapping.containsKey(openAPIType)) {
type = typeMapping.get(openAPIType);
if (languageSpecificPrimitives.contains(type)) {
return type;
}
} else {
type = openAPIType;
String type = super.getSchemaType(p);
if (typeMapping.containsKey(type)) {
return typeMapping.get(type);
}
if (languageSpecificPrimitives.contains(type)) {
return type;
}
return toModelName(type);
}
Expand All @@ -479,78 +507,35 @@ public Map<String, Object> postProcessModels(Map<String, Object> objs) {
}

@Override
public Map<String, Object> postProcessModelsEnum(Map<String, Object> objs) {
List<Object> models = (List<Object>) objs.get("models");
for (Object _mo : models) {
Map<String, Object> mo = (Map<String, Object>) _mo;
CodegenModel cm = (CodegenModel) mo.get("model");
boolean succes = buildEnumFromVendorExtension(cm) ||
buildEnumFromValues(cm);
for (CodegenProperty var : cm.vars) {
updateCodegenPropertyEnum(var);
}
}
return objs;
}

/**
* Builds the set of enum members from their declared value.
*
* @return {@code true} if the enum was built
*/
private boolean buildEnumFromValues(CodegenModel cm) {
if (!cm.isEnum || cm.allowableValues == null) {
return false;
}
Map<String, Object> allowableValues = cm.allowableValues;
List<Object> values = (List<Object>) allowableValues.get("values");
List<Map<String, Object>> enumVars = buildEnumVars(values, cm.dataType);
cm.allowableValues.put("enumVars", enumVars);
return true;
}

/**
* Builds the set of enum members from a vendor extension.
*
* @return {@code true} if the enum was built
*/
private boolean buildEnumFromVendorExtension(CodegenModel cm) {
if (!cm.isEnum || cm.allowableValues == null ||
!useEnumExtension ||
!cm.vendorExtensions.containsKey("x-enum-values")) {
return false;
}
Object extension = cm.vendorExtensions.get("x-enum-values");
List<Map<String, Object>> values = (List<Map<String, Object>>) extension;
List<Map<String, String>> enumVars = new ArrayList<>();
for (Map<String, Object> value : values) {
Map<String, String> enumVar = new HashMap<>();
String name = camelize((String) value.get("identifier"), true);
if (isReservedWord(name)) {
name = escapeReservedWord(name);
}
enumVar.put("name", name);
enumVar.put("value", toEnumValue(value.get("numericValue").toString(), cm.dataType));
if (value.containsKey("description")) {
enumVar.put("description", value.get("description").toString());
protected void updateEnumVarsWithExtensions(List<Map<String, Object>> enumVars, Map<String, Object> vendorExtensions, String dataType) {
if (vendorExtensions != null && useEnumExtension && vendorExtensions.containsKey("x-enum-values")) {
// Use the x-enum-values extension for this enum
// Existing enumVars added by the default handling need to be removed first
enumVars.clear();

Object extension = vendorExtensions.get("x-enum-values");
List<Map<String, Object>> values = (List<Map<String, Object>>) extension;
for (Map<String, Object> value : values) {
Map<String, Object> enumVar = new HashMap<>();
enumVar.put("name", toEnumVarName((String) value.get("identifier"), dataType));
enumVar.put("value", toEnumValue(value.get("numericValue").toString(), dataType));
enumVar.put("isString", isDataTypeString(dataType));
if (value.containsKey("description")) {
enumVar.put("description", value.get("description").toString());
}
enumVars.add(enumVar);
}
enumVars.add(enumVar);
} else {
super.updateEnumVarsWithExtensions(enumVars, vendorExtensions, dataType);
}
cm.allowableValues.put("enumVars", enumVars);
return true;
}

@Override
public String toEnumVarName(String value, String datatype) {
if (value.length() == 0) {
return "empty";
}
String var = value.replaceAll("\\W+", "_");
if ("number".equalsIgnoreCase(datatype) ||
"int".equalsIgnoreCase(datatype)) {
var = "Number" + var;
}
return escapeReservedWord(camelize(var, true));
return toVarName(value, "number");
}

@Override
Expand All @@ -565,14 +550,24 @@ public String toEnumValue(String value, String datatype) {

@Override
public String toOperationId(String operationId) {
operationId = super.toOperationId(operationId);

operationId = camelize(sanitizeName(operationId), true);

// method name cannot use reserved keyword, e.g. return
if (isReservedWord(operationId)) {
String newOperationId = camelize("call_" + operationId, true);
LOGGER.warn(operationId + " (reserved word) cannot be used as method name. Renamed to " + newOperationId);
return newOperationId;
}

return camelize(operationId, true);
// operationId starts with a number
if (operationId.matches("^\\d.*")) {
LOGGER.warn(operationId + " (starting with a number) cannot be used as method name. Renamed to " + camelize("call_" + operationId), true);
operationId = camelize("call_" + operationId, true);
}

return operationId;
}

public void setPubLibrary(String pubLibrary) {
Expand Down
Loading

0 comments on commit 4ad6d0b

Please sign in to comment.