Skip to content

Commit

Permalink
[csharp] ctor params should always be camelCase (#7519)
Browse files Browse the repository at this point in the history
* [csharp] ctor params should always be camelCase

After PR #6305, var names defaulted to PascalCase results in constructor
arguments also being PacalCase. Model properties and constructor
arguments have no reason to be the same case, and in fact may cause
issues (`name = name` will result in a compilation error).

This commit forces all constructor params in models to lowerCase.

This is a necessary change, for instance, if client SDK consumers assign
using named args:

var a = new Model(first = "", second = "")

The PacalCase default and update to constructor arg casing will break
existing consumers of the client.

See #7070 for more details and discussion.

* [csharp] Regenerate samples

* [csharp] Remove client models generated from a different spec.

* [csharp] Escape reserved words on camelcase/lowercase lambdas

* [csharp] Regenerate samples
  • Loading branch information
jimschubert authored and wing328 committed Feb 6, 2018
1 parent 1139f3f commit 0e34bcf
Show file tree
Hide file tree
Showing 159 changed files with 1,368 additions and 1,383 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -216,4 +216,6 @@ public interface CodegenConfig {

String toGetter(String name);

String sanitizeName(String name);

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package io.swagger.codegen.languages;

import com.google.common.collect.ImmutableMap;
import com.samskivert.mustache.Mustache;
import io.swagger.codegen.*;
import io.swagger.codegen.mustache.*;
import io.swagger.codegen.utils.ModelUtils;
import io.swagger.models.properties.*;
import org.apache.commons.lang3.StringUtils;
Expand All @@ -21,7 +24,7 @@ public abstract class AbstractCSharpCodegen extends DefaultCodegen implements Co
protected boolean returnICollection = false;
protected boolean netCoreProjectFileFlag = false;

protected String modelPropertyNaming = "PascalCase";
protected String modelPropertyNaming = CodegenConstants.MODEL_PROPERTY_NAMING_TYPE.PascalCase.name();

protected String packageVersion = "1.0.0";
protected String packageName = "IO.Swagger";
Expand Down Expand Up @@ -305,6 +308,32 @@ public void processOpts() {

// This either updates additionalProperties with the above fixes, or sets the default if the option was not specified.
additionalProperties.put(CodegenConstants.INTERFACE_PREFIX, interfacePrefix);

addMustacheLambdas(additionalProperties);
}

private void addMustacheLambdas(Map<String, Object> objs) {

Map<String, Mustache.Lambda> lambdas = new ImmutableMap.Builder<String, Mustache.Lambda>()
.put("lowercase", new LowercaseLambda().generator(this))
.put("uppercase", new UppercaseLambda())
.put("titlecase", new TitlecaseLambda())
.put("camelcase", new CamelCaseLambda().generator(this))
.put("camelcase_param", new CamelCaseLambda().generator(this).escapeAsParamName(true))
.put("indented", new IndentedLambda())
.put("indented_8", new IndentedLambda(8, " "))
.put("indented_12", new IndentedLambda(12, " "))
.put("indented_16", new IndentedLambda(16, " "))
.build();

if (objs.containsKey("lambda")) {
LOGGER.warn("An property named 'lambda' already exists. Mustache lambdas renamed from 'lambda' to '_lambda'. " +
"You'll likely need to use a custom template, " +
"see https://github.com/swagger-api/swagger-codegen#modifying-the-client-library-format. ");
objs.put("_lambda", lambdas);
} else {
objs.put("lambda", lambdas);
}
}

@Override
Expand Down Expand Up @@ -351,7 +380,7 @@ public Map<String, Object> postProcessAllModels(Map<String, Object> objs) {
* When working with enums, we can't always assume a RefModel is a nullable type (where default(YourType) == null),
* so this post processing runs through all models to find RefModel'd enums. Then, it runs through all vars and modifies
* those vars referencing RefModel'd enums to work the same as inlined enums rather than as objects.
* @param models
* @param models processed models to be further processed for enum references
*/
@SuppressWarnings({ "unchecked" })
private void postProcessEnumRefs(final Map<String, Object> models) {
Expand Down Expand Up @@ -750,8 +779,8 @@ public String getSwaggerType(Property p) {

/**
* Provides C# strongly typed declaration for simple arrays of some type and arrays of arrays of some type.
* @param arr
* @return
* @param arr The input array property
* @return The type declaration when the type is an array of arrays.
*/
private String getArrayTypeDeclaration(ArrayProperty arr) {
// TODO: collection type here should be fully qualified namespace to avoid model conflicts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,19 @@

import com.google.common.collect.ImmutableMap;
import com.samskivert.mustache.Mustache;
import com.samskivert.mustache.Template;
import io.swagger.codegen.*;

import java.io.File;
import java.io.IOException;
import java.io.Writer;
import java.util.*;

import io.swagger.codegen.mustache.IndentedLambda;
import io.swagger.codegen.mustache.LowercaseLambda;
import io.swagger.codegen.mustache.TitlecaseLambda;
import io.swagger.codegen.mustache.UppercaseLambda;
import org.apache.commons.lang3.StringUtils;
import io.swagger.codegen.CliOption;
import io.swagger.codegen.CodegenConstants;
import io.swagger.codegen.CodegenType;
import io.swagger.codegen.SupportingFile;
import io.swagger.codegen.mustache.*;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.File;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

public class KotlinServerCodegen extends AbstractKotlinCodegen {

public static final String DEFAULT_LIBRARY = Constants.KTOR;
Expand Down Expand Up @@ -191,9 +188,10 @@ public void processOpts() {
private void addMustacheLambdas(Map<String, Object> objs) {

Map<String, Mustache.Lambda> lambdas = new ImmutableMap.Builder<String, Mustache.Lambda>()
.put("lowercase", new LowercaseLambda())
.put("lowercase", new LowercaseLambda().generator(this))
.put("uppercase", new UppercaseLambda())
.put("titlecase", new TitlecaseLambda())
.put("camelcase", new CamelCaseLambda().generator(this))
.put("indented", new IndentedLambda())
.put("indented_8", new IndentedLambda(8, " "))
.put("indented_12", new IndentedLambda(12, " "))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package io.swagger.codegen.mustache;

import com.google.common.base.CaseFormat;
import com.samskivert.mustache.Mustache;
import com.samskivert.mustache.Template;
import io.swagger.codegen.CodegenConfig;
import io.swagger.codegen.DefaultCodegen;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.text.WordUtils;

import java.io.IOException;
import java.io.Writer;

/**
* Converts text in a fragment to camelCase.
*
* Register:
* <pre>
* additionalProperties.put("camelcase", new CamelCaseLambda());
* </pre>
*
* Use:
* <pre>
* {{#camelcase}}{{name}}{{/camelcase}}
* </pre>
*/
public class CamelCaseLambda implements Mustache.Lambda {
private CodegenConfig generator = null;
private Boolean escapeParam = false;

public CamelCaseLambda() {

}

public CamelCaseLambda generator(final CodegenConfig generator) {
this.generator = generator;
return this;
}

public CamelCaseLambda escapeAsParamName(final Boolean escape) {
this.escapeParam = escape;
return this;
}

@Override
public void execute(Template.Fragment fragment, Writer writer) throws IOException {
String text = DefaultCodegen.camelize(fragment.execute(), true);
if (generator != null) {
text = generator.sanitizeName(text);
if (generator.reservedWords().contains(text)) {
// Escaping must be done *after* camelize, because generators may escape using characters removed by camelize function.
text = generator.escapeReservedWord(text);
}

if (escapeParam) {
// NOTE: many generators call escapeReservedWord in toParamName, but we can't assume that's always the case.
// Here, we'll have to accept that we may be duplicating some work.
text = generator.toParamName(text);
}
}
writer.write(text);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.samskivert.mustache.Mustache;
import com.samskivert.mustache.Template;
import io.swagger.codegen.CodegenConfig;

import java.io.IOException;
import java.io.Writer;
Expand All @@ -20,10 +21,24 @@
* </pre>
*/
public class LowercaseLambda implements Mustache.Lambda {
private CodegenConfig generator = null;

public LowercaseLambda() {

}

public LowercaseLambda generator(final CodegenConfig generator) {
this.generator = generator;
return this;
}

@Override
public void execute(Template.Fragment fragment, Writer writer) throws IOException {
String text = fragment.execute();
writer.write(text.toLowerCase());
String text = fragment.execute().toLowerCase();
if (generator != null && generator.reservedWords().contains(text)) {
text = generator.escapeReservedWord(text);
}
writer.write(text);

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,26 @@
/// </summary>
{{#vars}}
{{^isReadOnly}}
/// <param name="{{name}}">{{#description}}{{description}}{{/description}}{{^description}}{{name}}{{/description}}{{#required}} (required){{/required}}{{#defaultValue}} (default to {{defaultValue}}){{/defaultValue}}.</param>
/// <param name="{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}}">{{#description}}{{description}}{{/description}}{{^description}}{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}}{{/description}}{{#required}} (required){{/required}}{{#defaultValue}} (default to {{defaultValue}}){{/defaultValue}}.</param>
{{/isReadOnly}}
{{/vars}}
{{#hasOnlyReadOnly}}
[JsonConstructorAttribute]
{{/hasOnlyReadOnly}}
public {{classname}}({{#readWriteVars}}{{{datatypeWithEnum}}}{{#isEnum}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}}{{/isEnum}} {{name}} = {{#defaultValue}}{{{defaultValue}}}{{/defaultValue}}{{^defaultValue}}default({{{datatypeWithEnum}}}{{#isEnum}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}}{{/isEnum}}){{/defaultValue}}{{^-last}}, {{/-last}}{{/readWriteVars}}){{#parent}} : base({{#parentVars}}{{name}}{{#hasMore}}, {{/hasMore}}{{/parentVars}}){{/parent}}
public {{classname}}({{#readWriteVars}}{{{datatypeWithEnum}}}{{#isEnum}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}}{{/isEnum}} {{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}} = {{#defaultValue}}{{{defaultValue}}}{{/defaultValue}}{{^defaultValue}}default({{{datatypeWithEnum}}}{{#isEnum}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}}{{/isEnum}}){{/defaultValue}}{{^-last}}, {{/-last}}{{/readWriteVars}}){{#parent}} : base({{#parentVars}}{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}}{{#hasMore}}, {{/hasMore}}{{/parentVars}}){{/parent}}
{
{{#vars}}
{{^isInherited}}
{{^isReadOnly}}
{{#required}}
// to ensure "{{name}}" is required (not null)
if ({{name}} == null)
// to ensure "{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}}" is required (not null)
if ({{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}} == null)
{
throw new InvalidDataException("{{name}} is a required property for {{classname}} and cannot be null");
throw new InvalidDataException("{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}} is a required property for {{classname}} and cannot be null");
}
else
{
this.{{name}} = {{name}};
this.{{name}} = {{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}};
}
{{/required}}
{{/isReadOnly}}
Expand All @@ -78,18 +78,18 @@
{{^isInherited}}
{{^isReadOnly}}
{{^required}}
{{#defaultValue}}// use default value if no "{{name}}" provided
if ({{name}} == null)
{{#defaultValue}}// use default value if no "{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}}" provided
if ({{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}} == null)
{
this.{{name}} = {{{defaultValue}}};
}
else
{
this.{{name}} = {{name}};
this.{{name}} = {{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}};
}
{{/defaultValue}}
{{^defaultValue}}
this.{{name}} = {{name}};
this.{{name}} = {{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}};
{{/defaultValue}}
{{/required}}
{{/isReadOnly}}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package io.swagger.codegen.mustache;

import io.swagger.codegen.CodegenConfig;
import io.swagger.codegen.languages.CSharpClientCodegen;
import io.swagger.codegen.languages.ScalaClientCodegen;
import org.testng.annotations.Factory;
import org.testng.annotations.Test;

import static org.testng.Assert.*;

public class CamelCaseLambdaTest extends MustacheTestBase {
private final String input;
private final String expected;
private Boolean escapeAsParamName = false;

private CodegenConfig generator = null;

public CamelCaseLambdaTest(String input, String expected) {
this.input = input;
this.expected = expected;
}

public CamelCaseLambdaTest generator(CodegenConfig generator) {
this.generator = generator;
return this;
}

public CamelCaseLambdaTest escapeAsParamName(Boolean setting) {
this.escapeAsParamName = setting;
return this;
}

@Test(description = "camelCase expected inputs")
public void testExecute() throws Exception {
// Arrange
String template = "{{#camelcase}}{{value}}{{/camelcase}}";
Object inputCtx = context(
"camelcase", new CamelCaseLambda().generator(this.generator).escapeAsParamName(this.escapeAsParamName),
"value", this.input
);

// Act
String actual = compile(template, inputCtx);


// Assert
assertEquals(actual, this.expected);
}

@Factory
public static Object[] factoryMethod() {
return new Object[] {
new CamelCaseLambdaTest("lowercase input", "lowercase input"),

// NOTE: DefaultCodegen.camelize(string, true) only results in first character of first word being lowercased.
// Keeping this behavior as it will match whatever is expected by existing codegen implementations.
new CamelCaseLambdaTest("UPPERCASE INPUT", "uPPERCASE INPUT"),
new CamelCaseLambdaTest("inputText", "inputText"),
new CamelCaseLambdaTest("input_text", "inputText"),

// TODO: This result for INPUT_TEXT may be unexpected, but is the result of DefaultCodegen.camelize.
// CamelCaseLambda can be extended to accept a method reference after move to Java 8.
new CamelCaseLambdaTest("INPUT_TEXT", "iNPUTTEXT"),
new CamelCaseLambdaTest("input-text", "inputText"),
new CamelCaseLambdaTest("input-text input-text input-text input-text input-text", "inputText inputText inputText inputText inputText"),
// C# codegen at time of writing this test escapes using a character that would be removed by camelize function.
new CamelCaseLambdaTest("class", "_class").generator(new CSharpClientCodegen()),
new CamelCaseLambdaTest("123List", "_123List").generator(new CSharpClientCodegen()).escapeAsParamName(true),
// Scala codegen is only one at time of writing this test that uses a Mustache.Escaper
new CamelCaseLambdaTest("class", "`class`").generator(new ScalaClientCodegen())
};
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.swagger.codegen.mustache;

import io.swagger.codegen.languages.CSharpClientCodegen;
import org.testng.annotations.Test;

import static org.testng.Assert.*;
Expand Down Expand Up @@ -28,4 +29,21 @@ public void testExecute() throws Exception {
assertEquals(lowercaseResult, "lowercase input");
assertEquals(uppercaseResult, "uppercase input");
}

@Test(description = "lowercase escapes reserved words")
public void testEscapingReservedWords() {
// Arrange
String template = "{{#lowercase}}{{value}}{{/lowercase}}";
String expected = "_class";
Object ctx = context(
"lowercase", new LowercaseLambda().generator(new CSharpClientCodegen()),
"value", "CLASS"
);

// Act
String actual = compile(template, ctx);

// Assert
assertEquals(actual, expected);
}
}
Loading

0 comments on commit 0e34bcf

Please sign in to comment.