swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
17.02k stars 6.03k forks source link

CSharp language generator results in code with ambigious references #5577

Open mminns opened 7 years ago

mminns commented 7 years ago
Description

The CSharp api..mustache template creates API classes that reference Model objects. However if the Model contains classes with the same name as core C# classes the code will not compile without the references being explicitly specified.

Swagger-codegen version

Built from master @ commit ea16da8

Swagger declaration file content or url

https://api.bitbucket.org/swagger.json

Command line used for generation

java -jar C:\Users\mminn\Source\github.com\mminns\swagger-codegen\modules\swagger-codegen-cli\target\swagger-codegen-cli.jar generate -i swagger.json -l csharp -o client-mminns/csharp

Steps to reproduce
  1. Run the generator
  2. Open the generated sln in Visual Studio
  3. Try and compile

Results:

Compilation fails Open Issue_trackerApi.cs

Listed with multiple errors, e.g.

Severity Code Description Project File Line Suppression State Error CS0104 'Version' is an ambiguous reference between 'IO.Swagger.Model.Version' and 'System.Version' IO.Swagger C:\Users\mminn\Source\bitbucket.org\mminns\bitbuckit.net.swagger\client-mminnns\csharp-debug\src\IO.Swagger\Api\Issue_trackerApi.cs 562 Active

Related issues

https://github.com/swagger-api/swagger-codegen/issues/5576

Suggest a Fix

In https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/AbstractCSharpCodegen.java#L310

Add the following method:

+    private void processUsings(Map<String, Object> objs) {
+        List<Map<String, String>> imports = (List<Map<String, String>>)objs.get("imports");
+        Map<String,String> usings = new HashMap<String,String>();
+        for(Map<String, String> importMap: imports) {
+            for(Map.Entry<String, String> entry: importMap.entrySet()) {
+                String fullModelName = entry.getValue();
+                String[] parts = fullModelName.split("\\.");
+                usings.put(parts[1], fullModelName);
+            }
+        }
+        objs.put("usings", usings.entrySet());
+    }

And using it in:

    @Override
    public Map<String, Object> postProcessModels(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");
            for (CodegenProperty var : cm.vars) {
                // check to see if model name is same as the property name
                // which will result in compilation error
                // if found, prepend with _ to workaround the limitation
                if (var.name.equalsIgnoreCase(cm.name)) {
                    var.name = "_" + var.name;
                }
            }
        }
+
+        processUsings(objs);

        // process enum in models
        return postProcessModelsEnum(objs);
    }

The in https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/csharp/api.mustache#L15

Add the following to make use of the new "usings" data

{{>partial_header}}
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
{{#netStandard}}
using RestSharp.Portable;
{{/netStandard}}
{{^netStandard}}
using RestSharp;
{{/netStandard}}
using {{packageName}}.Client;
{{#hasImport}}using {{packageName}}.{{modelPackage}};
{{/hasImport}}
+{{#usings}}using {{key}} = {{packageName}}.{{value}};
+{{/usings}}

namespace {{packageName}}.{{apiPackage}}
{
    {{#operations}}
    /// <summary>

see also https://github.com/mminns/swagger-codegen/tree/issue/fix-csharp-for-bitbucket\

I'd like to submit a PR for the changes

jimschubert commented 7 years ago

@mminns This is probably a silly question, but why not use {{modelPackage}} as a prefix in code when you're working with a type that is neither primitive nor container type?

Something like this for operation return types:

{{#returnType}}{{#isNotContainer}}{{^isPrimitiveType}}{{modelPackage}}.{{{returnType}}}{{/isPrimitiveType}}{{#isPrimitiveType}}{{{returnType}}}{{/isPrimitiveType}}{{/isNotContainer}}{{#isContainer}}{{{returnType}}}{{/isContainer}{{/returnType}}

It's ugly, but I think this would be simpler than a lot of shuffling code around within the generator. I think you'd only need to have this wacky long string on method inputs/outputs. Model properties should be safe, unless there are models named something likeInt64 or whatever (sorry, I didn't look at the spec you're targeting).

Another option could be to set a vendor extension like I do for some complex tweaks in the finch generator. see FinchServerGenerator#postProcessOperations. This would have to be done for operations.

Also, I did some work on a new Kotlin generator in which I fully qualify kotlin.* primitives, collections used by the generator, and a handful of common Java types. What I did there may be helpful to you: https://github.com/swagger-api/swagger-codegen/pull/5727/files See getSwaggerType, getTypeDeclaration, needToImport, and toModelImport.

mminns commented 7 years ago

Sorry missed your comment.

I will test that as an alternative see if that works. In effect its not that much different to the solution in https://github.com/swagger-api/swagger-codegen/pull/5682/files which just adds an explicit returnType package value to the data passed tot he template.

kevineger commented 6 years ago

+1