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

C# generator includes unnecessary usings in Operations files, causing compilation errors #2558

Closed
matthchr opened this issue Aug 29, 2017 · 6 comments
Assignees

Comments

@matthchr
Copy link
Member

In our IOperations classes, when using an up to date version of AutoRest, I am seeing this change:

-    using System.Linq;
+    using Microsoft.Azure;
+    using Microsoft.Azure.Batch;
     using Microsoft.Rest;
     using Microsoft.Rest.Azure;
+    using Microsoft.Rest.Serialization;
     using Models;
+    using Newtonsoft.Json;
+    using System.Collections;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Net;
+    using System.Net.Http;
+    using System.Threading;
+    using System.Threading.Tasks;

The issue is that using Microsoft.Azure and using Microsoft.Azure.Batch were added. By adding these usings (which aren't needed by the way), we now have ambiguous references in the generated code. Specifically, in the method signature: public async Task<AzureOperationResponse<IPage<NodeAgentSku>,AccountListNodeAgentSkusHeaders>> the NodeAgentSku type is flagged as duplicate because it exists in both the Microsoft.Azure.Batch namespace, and the Models namespace.

This paradigm has worked for us since we started using AutoRest -- is there a way to configure the code generator to omit the Microsoft.Azure.Batch using statement (which is causing these issues and isn't needed)?

@fearthecowboy
Copy link
Member

Hmm.

How long ago (ie, what autorest version) was the last code generation made?

I'm surprised that it produces code that is ambiguous since it's a Roslyn based simplifier. If there is a conflict, it should have opted to use the full class names.

@matthchr
Copy link
Member Author

We were using this previously: 1.0.0-Nightly20170209

You can run this file: https://github.com/Azure/azure-sdk-for-net/blob/psSdkJson6/src/SDKs/Batch/DataPlane/generate.cmd and reproduce the issue.

Maybe the fact that the conflicts are coming from the .sln which includes directories above the generated directory is the problem?

@fearthecowboy
Copy link
Member

The SLN shouldn't be the problem. File selection happens from the csproj file.

That looks like it's probably a build from before we moved from Roslyn 1.3 to 2.0.

I'm going to have to dig into this-- the simplifier shouldn't be able to produce code that doesn't compile.

Can you add --disable-simplifier and see if those are still generated?

@fearthecowboy
Copy link
Member

Yeah, this was the result of us bumping the Roslyn bits. It's "helping" by adding in explicit references when there are implicit references in place (ie, if your namespace is foo.bar.baz it adds usings for foo.bar and foo -- which are implicit (and implicit used namespaces don't cause ambiguity)

I'm going to add a directive to tweak that, it's going to show up in the 2.0 stable release in september.

I can give you a scripting workaround for now...

@fearthecowboy fearthecowboy self-assigned this Aug 30, 2017
matthchr added a commit to matthchr/azure-sdk-for-net that referenced this issue Sep 8, 2017
  - There is currently a bug Azure/autorest#2558 which had to be worked around
    with a hacky script fix, in fixup.ps1. Once that issue is fixed we should remove fixup.ps1.
shahabhijeet pushed a commit to Azure/azure-sdk-for-net that referenced this issue Sep 12, 2017
…3679)

* Update to latest version of AutoRest code generator

  - There is currently a bug Azure/autorest#2558 which had to be worked around
    with a hacky script fix, in fixup.ps1. Once that issue is fixed we should remove fixup.ps1.

* Remove PORTABLE from Batch.csproj
@dsgouda dsgouda assigned dsgouda and unassigned fearthecowboy Sep 12, 2017
@olydis
Copy link
Contributor

olydis commented Sep 18, 2017

@dsgouda closing, I assume you have confirmed that the above specific case is fixed? 🙂
@matthchr this will be available with the next AutoRest release (right after Ignite)

@dsgouda
Copy link
Contributor

dsgouda commented Sep 18, 2017

Yes sir!

@dsgouda dsgouda closed this as completed Sep 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants