search

facebookarchive / swift

An annotation-based Java library for creating Thrift serializable types and services.
Apache License 2.0
900 stars 297 forks source link

Swift generator Constants file confict if more than one file in the same namespace defines a contant #152

Closed l3fang closed 10 years ago

l3fang commented 10 years ago

ava -jar /tmp/swift-generator-cli-0.10.0-standalone.jar common/smc/if/Smc2.thrift

Smc2.thrift defines a constant but itself imports ServiceManager.thrift. ServiceManager.thrift defines more constants. The generator creates Constants.java for Smc2.thrift and ServiceManager.thrift separately. It requires a developer to manually rename one of the Constants file to a different name, otherwise the build will fail with two classes having the same name.

andrewcox commented 10 years ago

Agree this could work better, if we'd generate constants that are in the same namespace in included .thrift files at the same time.

Workaround: generate both Smc2.thrift and ServiceManager.thrift on the same swift-generator-cli command line, then all constants will be in the same Constants.java file.

andrewcox commented 10 years ago

BTW the problem isn't unique to swift, standard thrift generator has the same problem, which you can workaround the same way.

andrewcox commented 10 years ago

(should have said "has the same problem for java codegen")

l3fang commented 10 years ago

The issue is I wish to put the two interfaces in two separate modules. It seems the smc team gave a better workaround here: to generate Smc2.thrift in the com.facebook.smc2 package. I feel this is a better package name for the interface as well.

andrewcox commented 10 years ago

Yeah, if you want to split them into two modules, there is no way to do it without using different namespaces or to generate separate classes for each .thrift file (e.g. ServiceManagerConstants and Smc2Constants) - but this would be a breaking change and I also don't like the idea of naming classes after .thrift file names as there isn't really any precedent for this and some thrift filenames in use wouldn't make good class names – e.g. common.thrift)

Using different namespaces seems like the right way to deal with this.

If you've got an idea of how we could better handle this in swift, let me know, otherwise we should close this as won't fix, and just require different namespaces if you are using constants.

From: Li Fang notifications@github.com<mailto:notifications@github.com> Reply-To: facebook/swift reply@reply.github.com<mailto:reply@reply.github.com> Date: Wednesday, November 13, 2013 8:44 AM To: facebook/swift swift@noreply.github.com<mailto:swift@noreply.github.com> Cc: Andrew Cox andrewcox@fb.com<mailto:andrewcox@fb.com> Subject: Re: [swift] Swift generator Constants file confict if more than one file in the same namespace defines a contant (#152)

The issue is I wish to put the two interfaces in two separate modules. It seems the smc team gave a better workaround here: to generate Smc2.thrift in the com.facebook.smc2 package. I feel this is a better package name for the interface as well.

— Reply to this email directly or view it on GitHubhttps://urldefense.proofpoint.com/v1/url?u=https://github.com/facebook/swift/issues/152%23issuecomment-28410817&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=sJ61cr6u3yYT6%2FsOZjgMEw%3D%3D%0A&m=E2v83FCCKh2GwKOYL0G4wGXXP%2F0jSOAJME53EeyvGAM%3D%0A&s=5cf4485b5e740ba3459ac6af00414cf94a4876cce36a87744b1c849cc4648cc2.

l3fang commented 10 years ago

I feel this is fair assessment. I will close it as not a bug.