Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[scala] [template] scala model property style #5486

Merged
merged 5 commits into from
Mar 2, 2020
Merged
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
1 change: 1 addition & 0 deletions docs/generators/scala-akka.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ sidebar_label: scala-akka
|ensureUniqueParams|Whether to ensure parameter names are unique in an operation (rename parameters that are not).| |true|
|mainPackage|Top-level package name, which defines 'apiPackage', 'modelPackage', 'invokerPackage'| |org.openapitools.client|
|modelPackage|package for generated models| |null|
|modelPropertyNaming|Naming convention for the property: 'camelCase', 'PascalCase', 'snake_case' and 'original', which keeps the original name| |camelCase|
|prependFormOrBodyParameters|Add form or body parameters to the beginning of the parameter list.| |false|
|sortModelPropertiesByRequiredFlag|Sort model properties to place required parameters before optional parameters.| |true|
|sortParamsByRequiredFlag|Sort method arguments to place required parameters before optional parameters.| |true|
Expand Down
1 change: 1 addition & 0 deletions docs/generators/scala-gatling.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ sidebar_label: scala-gatling
|apiPackage|package for generated api classes| |null|
|ensureUniqueParams|Whether to ensure parameter names are unique in an operation (rename parameters that are not).| |true|
|modelPackage|package for generated models| |null|
|modelPropertyNaming|Naming convention for the property: 'camelCase', 'PascalCase', 'snake_case' and 'original', which keeps the original name| |camelCase|
|prependFormOrBodyParameters|Add form or body parameters to the beginning of the parameter list.| |false|
|sortModelPropertiesByRequiredFlag|Sort model properties to place required parameters before optional parameters.| |true|
|sortParamsByRequiredFlag|Sort method arguments to place required parameters before optional parameters.| |true|
Expand Down
1 change: 1 addition & 0 deletions docs/generators/scala-play-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ sidebar_label: scala-play-server
|ensureUniqueParams|Whether to ensure parameter names are unique in an operation (rename parameters that are not).| |true|
|generateCustomExceptions|If set, generates custom exception types.| |true|
|modelPackage|package for generated models| |null|
|modelPropertyNaming|Naming convention for the property: 'camelCase', 'PascalCase', 'snake_case' and 'original', which keeps the original name| |camelCase|
|prependFormOrBodyParameters|Add form or body parameters to the beginning of the parameter list.| |false|
|routesFileName|Name of the routes file to generate.| |routes|
|skipStubs|If set, skips generation of stub classes.| |false|
Expand Down
1 change: 1 addition & 0 deletions docs/generators/scala-sttp.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ sidebar_label: scala-sttp
|ensureUniqueParams|Whether to ensure parameter names are unique in an operation (rename parameters that are not).| |true|
|mainPackage|Top-level package name, which defines 'apiPackage', 'modelPackage', 'invokerPackage'| |org.openapitools.client|
|modelPackage|package for generated models| |null|
|modelPropertyNaming|Naming convention for the property: 'camelCase', 'PascalCase', 'snake_case' and 'original', which keeps the original name| |camelCase|
|prependFormOrBodyParameters|Add form or body parameters to the beginning of the parameter list.| |false|
|sortModelPropertiesByRequiredFlag|Sort model properties to place required parameters before optional parameters.| |true|
|sortParamsByRequiredFlag|Sort method arguments to place required parameters before optional parameters.| |true|
Expand Down
1 change: 1 addition & 0 deletions docs/generators/scalatra.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ sidebar_label: scalatra
|apiPackage|package for generated api classes| |null|
|ensureUniqueParams|Whether to ensure parameter names are unique in an operation (rename parameters that are not).| |true|
|modelPackage|package for generated models| |null|
|modelPropertyNaming|Naming convention for the property: 'camelCase', 'PascalCase', 'snake_case' and 'original', which keeps the original name| |camelCase|
|prependFormOrBodyParameters|Add form or body parameters to the beginning of the parameter list.| |false|
|sortModelPropertiesByRequiredFlag|Sort model properties to place required parameters before optional parameters.| |true|
|sortParamsByRequiredFlag|Sort method arguments to place required parameters before optional parameters.| |true|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@
import java.util.Map;

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

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

protected String modelPropertyNaming = "camelCase";
protected String modelPropertyNaming = CodegenConstants.ENUM_PROPERTY_NAMING_TYPE.camelCase.name();
protected String invokerPackage = "org.openapitools.client";
protected String sourceFolder = "src/main/scala";
protected boolean stripPackageName = true;
Expand Down Expand Up @@ -115,6 +116,7 @@ public AbstractScalaCodegen() {
cliOptions.add(new CliOption(CodegenConstants.MODEL_PACKAGE, CodegenConstants.MODEL_PACKAGE_DESC));
cliOptions.add(new CliOption(CodegenConstants.API_PACKAGE, CodegenConstants.API_PACKAGE_DESC));
cliOptions.add(new CliOption(CodegenConstants.SOURCE_FOLDER, CodegenConstants.SOURCE_FOLDER_DESC));
cliOptions.add(new CliOption(CodegenConstants.MODEL_PROPERTY_NAMING, CodegenConstants.MODEL_PROPERTY_NAMING_DESC).defaultValue(modelPropertyNaming));

}

Expand All @@ -137,6 +139,62 @@ public void processOpts() {
LOGGER.warn("stripPackageName=false. Compilation errors may occur if API type names clash with types " +
"in the default imports");
}
if (additionalProperties.containsKey(CodegenConstants.MODEL_PROPERTY_NAMING)) {
setModelPropertyNaming(
(String) additionalProperties.get(CodegenConstants.MODEL_PROPERTY_NAMING));
}
}

public void setModelPropertyNaming(String naming) {
try {
this.modelPropertyNaming = CodegenConstants.ENUM_PROPERTY_NAMING_TYPE.valueOf(naming).name();
} catch (IllegalArgumentException ex) {
throw new IllegalArgumentException("Invalid model property naming '" +
naming + "'. Must be 'original', 'camelCase', " +
"'PascalCase' or 'snake_case'");
}
}

public String getModelPropertyNaming() {
return this.modelPropertyNaming;
}


@Override
public String toVarName(String name) {
String varName = sanitizeName(name);

if ("_".equals(varName)) {
varName = "_u";
}

// if it's all uppper case, do nothing
if (!varName.matches("^[A-Z_0-9]*$")) {
varName = getNameUsingModelPropertyNaming(varName);
}

if (isReservedWord(varName) || varName.matches("^\\d.*")) {
varName = escapeReservedWord(varName);
}

return varName;
}

public String getNameUsingModelPropertyNaming(String name) {
switch (CodegenConstants.MODEL_PROPERTY_NAMING_TYPE.valueOf(getModelPropertyNaming())) {
case original:
return name;
case camelCase:
return camelize(name, true);
case PascalCase:
return camelize(name);
case snake_case:
return underscore(name);
default:
throw new IllegalArgumentException("Invalid model property naming '" +
name + "'. Must be 'original', 'camelCase', " +
"'PascalCase' or 'snake_case'");
}
}

public String getSourceFolder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,6 @@ public String toParamName(String name) {
return formatIdentifier(name, false);
}

@Override
public String toVarName(String name) {
return formatIdentifier(name, false);
Copy link
Member

Choose a reason for hiding this comment

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

Would it be an issue if formatIdentifier is no longer used in toVarName?

Copy link
Contributor Author

@chameleon82 chameleon82 Mar 1, 2020

Choose a reason for hiding this comment

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

As of this change it is not backward compatible to akka-http, as formatIdentifier and toVarName has some difference when original name in UPPERCASE.
uPPERCASE for formatIdentifier
UPPERCASE for toVarName (do not apply naming rules for uppercase names)

I've added some tests to cover those cases.
I could revert formatIdentifier for default camelCase to make it backward compatible for 4.3.0 release, should I?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wing328 i could add next code

    @Override
    @Deprecated//(since = "5.0.0", forRemoval = true)
    public String toVarName(String name) {
        if(CodegenConstants.ENUM_PROPERTY_NAMING_TYPE.camelCase.name().equals(this.getModelPropertyNaming())) {
            return formatIdentifier(name, false);
        } else {
           return super.toVarName(name);
        }
    }

with tests

        // Next cases will be UPPERCASE since 5.0.0
        Assert.assertEquals(scalaAkkaClientCodegen.toVarName("USERNAME"), "uSERNAME");
        Assert.assertEquals(scalaAkkaClientCodegen.toVarName("USER123NAME"), "uSER123NAME");
        Assert.assertEquals(scalaAkkaClientCodegen.toVarName("1AAAA"), "`1aAAA`");
        Assert.assertEquals(scalaAkkaClientCodegen.toVarName("1A"), "`1a`");

do we need this fallback?

Copy link
Member

Choose a reason for hiding this comment

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

As discussed let's consider this as a bug fix as it's fixing the inconsistent variable naming in different Scala generators.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

@Override
public String toEnumName(CodegenProperty property) {
return formatIdentifier(property.baseName, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,58 +156,13 @@ public ScalaHttpClientCodegen() {

instantiationTypes.put("array", "ListBuffer");
instantiationTypes.put("map", "HashMap");

cliOptions.add(new CliOption(CodegenConstants.MODEL_PROPERTY_NAMING, CodegenConstants.MODEL_PROPERTY_NAMING_DESC).defaultValue("camelCase"));
}

@Override
public void processOpts() {
LOGGER.warn("IMPORTANT: This generator (scala-http-client-deprecated) is no longer actively maintained and will be deprecated. " +
"PLease use 'scala-akka' generator instead.");

super.processOpts();
if (additionalProperties.containsKey(CodegenConstants.MODEL_PROPERTY_NAMING)) {
setModelPropertyNaming((String) additionalProperties.get(CodegenConstants.MODEL_PROPERTY_NAMING));
}
}

public void setModelPropertyNaming(String naming) {
if ("original".equals(naming) || "camelCase".equals(naming) ||
"PascalCase".equals(naming) || "snake_case".equals(naming)) {
this.modelPropertyNaming = naming;
} else {
throw new IllegalArgumentException("Invalid model property naming '" +
naming + "'. Must be 'original', 'camelCase', " +
"'PascalCase' or 'snake_case'");
}
}

public String getModelPropertyNaming() {
return this.modelPropertyNaming;
}

@Override
public String toVarName(String name) {
// sanitize name
name = sanitizeName(name); // FIXME: a parameter should not be assigned. Also declare the methods parameters as 'final'.

if ("_".equals(name)) {
name = "_u";
}

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

name = getNameUsingModelPropertyNaming(name);

// for reserved word or word starting with number, append _
if (isReservedWord(name) || name.matches("^\\d.*")) {
name = escapeReservedWord(name);
}

return name;
}

@Override
Expand All @@ -216,24 +171,6 @@ public String toParamName(String name) {
return toVarName(name);
}

public String getNameUsingModelPropertyNaming(String name) {
switch (CodegenConstants.MODEL_PROPERTY_NAMING_TYPE.valueOf(getModelPropertyNaming())) {
case original:
return name;
case camelCase:
return camelize(name, true);
case PascalCase:
return camelize(name);
case snake_case:
return underscore(name);
default:
throw new IllegalArgumentException("Invalid model property naming '" +
name + "'. Must be 'original', 'camelCase', " +
"'PascalCase' or 'snake_case'");
}

}

@Override
public CodegenType getTag() {
return CodegenType.CLIENT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,59 +120,11 @@ public ScalaLagomServerCodegen() {

instantiationTypes.put("array", "ListBuffer");
instantiationTypes.put("map", "HashMap");

cliOptions.add(new CliOption(CodegenConstants.MODEL_PROPERTY_NAMING,
CodegenConstants.MODEL_PROPERTY_NAMING_DESC).defaultValue("camelCase"));
}

@Override
public void processOpts() {
super.processOpts();

if (additionalProperties.containsKey(CodegenConstants.MODEL_PROPERTY_NAMING)) {
setModelPropertyNaming(
(String) additionalProperties.get(CodegenConstants.MODEL_PROPERTY_NAMING));
}
}

public void setModelPropertyNaming(String naming) {
if ("original".equals(naming) || "camelCase".equals(naming) ||
"PascalCase".equals(naming) || "snake_case".equals(naming)) {
this.modelPropertyNaming = naming;
} else {
throw new IllegalArgumentException("Invalid model property naming '" +
naming + "'. Must be 'original', 'camelCase', " +
"'PascalCase' or 'snake_case'");
}
}

public String getModelPropertyNaming() {
return this.modelPropertyNaming;
}

@Override
public String toVarName(String name) {
// sanitize name
name = sanitizeName(
name); // FIXME: a parameter should not be assigned. Also declare the methods parameters as 'final'.

if ("_".equals(name)) {
name = "_u";
}

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

name = getNameUsingModelPropertyNaming(name);

// for reserved word or word starting with number, append _
if (isReservedWord(name) || name.matches("^\\d.*")) {
name = escapeReservedWord(name);
}

return name;
}

@Override
Expand All @@ -181,24 +133,6 @@ public String toParamName(String name) {
return toVarName(name);
}

private String getNameUsingModelPropertyNaming(String name) {
switch (CodegenConstants.MODEL_PROPERTY_NAMING_TYPE.valueOf(getModelPropertyNaming())) {
case original:
return name;
case camelCase:
return camelize(name, true);
case PascalCase:
return camelize(name);
case snake_case:
return underscore(name);
default:
throw new IllegalArgumentException("Invalid model property naming '" +
name + "'. Must be 'original', 'camelCase', " +
"'PascalCase' or 'snake_case'");
}

}

@Override
public CodegenType getTag() {
return CodegenType.SERVER;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,55 +128,11 @@ public ScalazClientCodegen() {
instantiationTypes.put("map", "HashMap");

additionalProperties.put("fnEnumEntry", new EnumEntryLambda());

cliOptions.add(new CliOption(CodegenConstants.MODEL_PROPERTY_NAMING, CodegenConstants.MODEL_PROPERTY_NAMING_DESC).defaultValue("camelCase"));
}

@Override
public void processOpts() {
super.processOpts();
if (additionalProperties.containsKey(CodegenConstants.MODEL_PROPERTY_NAMING)) {
setModelPropertyNaming((String) additionalProperties.get(CodegenConstants.MODEL_PROPERTY_NAMING));
}
}

public void setModelPropertyNaming(String naming) {
if ("original".equals(naming) || "camelCase".equals(naming) ||
"PascalCase".equals(naming) || "snake_case".equals(naming)) {
this.modelPropertyNaming = naming;
} else {
throw new IllegalArgumentException("Invalid model property naming '" +
naming + "'. Must be 'original', 'camelCase', " +
"'PascalCase' or 'snake_case'");
}
}

public String getModelPropertyNaming() {
return this.modelPropertyNaming;
}

@Override
public String toVarName(String name) {
// sanitize name
name = sanitizeName(name); // FIXME: a parameter should not be assigned. Also declare the methods parameters as 'final'.

if ("_".equals(name)) {
name = "_u";
}

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

name = getNameUsingModelPropertyNaming(name);

// for reserved word or word starting with number, append _
if (isReservedWord(name) || name.matches("^\\d.*")) {
name = escapeReservedWord(name);
}

return name;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class ScalaAkkaClientOptionsProvider implements OptionsProvider {
public static final String ALLOW_UNICODE_IDENTIFIERS_VALUE = "false";
public static final String PREPEND_FORM_OR_BODY_PARAMETERS_VALUE = "true";
public static final String MAIN_PACKAGE_VALUE = "net.test";
public static final String MODEL_PROPERTY_NAMING = "camelCase";


@Override
Expand All @@ -51,6 +52,7 @@ public Map<String, String> createOptions() {
.put(CodegenConstants.ALLOW_UNICODE_IDENTIFIERS, ALLOW_UNICODE_IDENTIFIERS_VALUE)
.put(CodegenConstants.PREPEND_FORM_OR_BODY_PARAMETERS, PREPEND_FORM_OR_BODY_PARAMETERS_VALUE)
.put("mainPackage", MAIN_PACKAGE_VALUE)
.put(CodegenConstants.MODEL_PROPERTY_NAMING, MODEL_PROPERTY_NAMING)
.build();
}

Expand Down
Loading