diff --git a/java/README.txt b/java/README.txt index 58ccb88eb5..8bfaab0799 100644 --- a/java/README.txt +++ b/java/README.txt @@ -91,9 +91,11 @@ Micro version ============================ The runtime and generated code for MICRO_RUNTIME is smaller -because it does not include support for the descriptor, -reflection or extensions. Also, not currently supported -are packed repeated elements nor testing of java_multiple_files. +because it does not include support for the descriptor and +reflection, and enums are generated as integer constants in +the parent message or the file's outer class. Also, not +currently supported are packed repeated elements or +extensions. To create a jar file for the runtime and run tests invoke "mvn package -P micro" from the /java @@ -128,26 +130,24 @@ opt -> speed or space java_use_vector -> true or false java_package -> | java_outer_classname -> | - -opt: - This changes the code generation to optimize for speed, - opt=speed, or space, opt=space. When opt=speed this - changes the code generation for strings so that multiple - conversions to Utf8 are eliminated. The default value - is opt=space. - -java_use_vector: - Is a boolean flag either java_use_vector=true or - java_use_vector=false. When java_use_vector=true the - code generated for repeated elements uses - java.util.Vector and when java_use_vector=false the - java.util.ArrayList<> is used. When java.util.Vector - is used the code must be compiled with Java 1.3 and - when ArrayList is used Java 1.5 or above must be used. - The using javac the source parameter may be used to - control the version of the source: "javac -source 1.3". - You can also change the xml element for the - maven-compiler-plugin. Below is for 1.5 sources: +java_multiple_files -> true or false + +opt={speed,space} (default: space) + This changes the code generation to optimize for speed or + space. When opt=speed this changes the code generation + for strings so that multiple conversions to Utf8 are + eliminated. + +java_use_vector={true,false} (default: false) + This specifies the collection class for repeated elements. + If false, repeated elements use java.util.ArrayList<> and + the code must be compiled with Java 1.5 or above. If true, + repeated elements use java.util.Vector and the code can + be compiled with Java 1.3 or above. The 'source' + parameter of 'javac' may be used to control the version + of the source: "javac -source 1.3". You can also change + the xml element for the maven-compiler-plugin. + Below is for 1.5 sources: maven-compiler-plugin @@ -157,11 +157,8 @@ java_use_vector: - When compiling for 1.5 java_use_vector=false or not - present where the default value is false. - - And below would be for 1.3 sources note when changing - to 1.3 you must also set java_use_vector=true: + And below would be for 1.3 sources (note when changing + to 1.3 you must also set java_use_vector=true): maven-compiler-plugin @@ -171,93 +168,241 @@ java_use_vector: -java_package: - The allows setting/overriding the java_package option - and associates allows a package name for a file to - be specified on the command line. Overriding any - "option java_package xxxx" in the file. The default - if not present is to use the value from the package - statment or "option java_package xxxx" in the file. +java_package=| (no default) + This allows overriding the 'java_package' option value + for the given file from the command line. Use multiple + java_package options to override the option for multiple + files. The final Java package for each file is the value + of this command line option if present, or the value of + the same option defined in the file if present, or the + proto package if present, or the default Java package. + +java_outer_classname=| (no default) + This allows overriding the 'java_outer_classname' option + for the given file from the command line. Use multiple + java_outer_classname options to override the option for + multiple files. The final Java outer class name for each + file is the value of this command line option if present, + or the value of the same option defined in the file if + present, or the file name converted to CamelCase. This + outer class will nest all classes and integer constants + generated from file-scope messages and enums. + +java_multiple_files={true,false} (no default) + This allows overriding the 'java_multiple_files' option + in all source files and their imported files from the + command line. The final value of this option for each + file is the value defined in this command line option, or + the value of the same option defined in the file if + present, or false. This specifies whether to generate + package-level classes for the file-scope messages in the + same Java package as the outer class (instead of nested + classes in the outer class). File-scope enum constants + are still generated as integer constants in the outer + class. This affects the fully qualified references in the + Java code. NOTE: because the command line option + overrides the value for all files and their imported + files, using this option inconsistently may result in + incorrect references to the imported messages and enum + constants. + + +IMPORTANT: change of javamicro_out behavior: + +In previous versions, if the outer class name has not been +given explicitly, javamicro_out would not infer the outer +class name from the file name, and would skip the outer +class generation. This makes the compilation succeed only +if the source file contains a single message and no enums, +and the generated class for that message is placed at the +package level. To re-align with java_out, javamicro_out +will now always generate the outer class, inferring its +name from the file name if not given, as a container of the +message classes and enum constants. To keep any existing +single-message source file from causing the generation of +an unwanted outer class, you can set the option +java_multiple_files to true, either in the file or as a +command line option. -java_outer_classname: - This allows the setting/overriding of the outer - class name option and associates a class name - to a file. An outer class name is required and - must be specified if there are multiple messages - in a single proto file either in the proto source - file or on the command line. If not present the - no outer class name will be used. Below are a series of examples for clarification of the -various javamicro_out parameters using -src/test/proto/simple-data.proto: +various parameters and options. Assuming this file: + +src/proto/simple-data-protos.proto: + + package testprotobuf; + + message SimpleData { + optional fixed64 id = 1; + optional string description = 2; + optional bool ok = 3 [default = false]; + }; + +and the compiled protoc in the current working directory, +then a simple command line to compile this file would be: -package testprotobuf; +./protoc --javamicro_out=. src/proto/simple-data-protos.proto -message SimpleData { - optional fixed64 id = 1; - optional string description = 2; - optional bool ok = 3 [default = false]; -}; +This will create testprotobuf/SimpleDataProtos.java, which +has the following content (extremely simplified): + package testprotobuf; -Assuming you've only compiled and not installed protoc and -your current working directory java/, then a simple -command line to compile simple-data would be: + public final class SimpleDataProtos { + public static final class SimpleData + extends MessageMicro { + ... + } + } -../src/protoc --javamicro_out=. src/test/proto/simple-data.proto +The message SimpleData is compiled into the SimpleData +class, nested in the file's outer class SimpleDataProtos, +whose name is implicitly defined by the proto file name +"simple-data-protos". -This will create testprotobuf/SimpleData.java +The directory, aka Java package, testprotobuf is created +because on line 1 of simple-data-protos.proto is +"package testprotobuf;". If you wanted a different +package name you could use the java_package option in the +file: -The directory testprotobuf is created because on line 1 -of simple-data.proto is "package testprotobuf;". If you -wanted a different package name you could use the -java_package option command line sub-parameter: + option java_package = "my_package"; -../src/protoc '--javamicro_out=java_package=src/test/proto/simple-data.proto|my_package:.' src/test/proto/simple-data.proto +or in command line sub-parameter: + +./protoc '--javamicro_out=\ +java_package=src/proto/simple-data-protos.proto|my_package:\ +.' src/proto/simple-data-protos.proto Here you see the new java_package sub-parameter which itself needs two parameters the file name and the -package name, these are separated by "|". Now you'll -find my_package/SimpleData.java. +package name, these are separated by "|". The value set +in the command line overrides the value set in the file. +Now you'll find SimpleDataProtos.java in the my_package/ +directory. If you wanted to also change the optimization for speed you'd add opt=speed with the comma seperator as follows: -../src/protoc '--javamicro_out=opt=speed,java_package=src/test/proto/simple-data.proto|my_package:.' src/test/proto/simple-data.proto +./protoc '--javamicro_out=\ +opt=speed,\ +java_package=src/proto/simple-data-protos.proto|my_package: +.' src/proto/simple-data-protos.proto -Finally if you also wanted an outer class name you'd +If you also wanted a different outer class name you'd do the following: -../src/protoc '--javamicro_out=opt=speed,java_package=src/test/proto/simple-data.proto|my_package,java_outer_classname=src/test/proto/simple-data.proto|OuterName:.' src/test/proto/simple-data.proto +./protoc '--javamicro_out=\ +opt=speed,\ +java_package=src/proto/simple-data-protos.proto|my_package,\ +java_outer_classname=src/proto/simple-data-protos.proto|OuterName:\ +.' src/proto/simple-data-protos.proto + +Now you'll find my_package/OuterName.java and the +message class SimpleData nested in it. -Now you'll find my_packate/OuterName.java. +As mentioned java_package, java_outer_classname and +java_multiple_files may also be specified in the file. +In the example below we must define +java_outer_classname because otherwise the outer class +and one of the message classes will have the same name, +which is forbidden to prevent name ambiguity: -As mentioned java_package and java_outer_classname -may also be specified in the file. In the example -below we must define java_outer_classname because -there are multiple messages in -src/test/proto/two-messages.proto +src/proto/sample-message.proto: -package testmicroruntime; + package testmicroruntime; -option java_package = "com.example"; -option java_outer_classname = "TestMessages"; + option java_package = "com.example"; + option java_outer_classname = "SampleMessageProtos"; -message TestMessage1 { - required int32 id = 1; -} + enum MessageType { + SAMPLE = 1; + EXAMPLE = 2; + } -message TestMessage2 { - required int32 id = 1; -} + message SampleMessage { + required int32 id = 1; + required MessageType type = 2; + } + + message SampleMessageContainer { + required SampleMessage message = 1; + } This could be compiled using: -../src/protoc --javamicro_out=. src/test/proto/two-message.proto +./protoc --javamicro_out=. src/proto/sample-message.proto + +and the output will be: + +com/example/SampleMessageProtos.java: + + package com.example; + + public final class SampleMessageProtos { + public static final int SAMPLE = 1; + public static final int EXAMPLE = 2; + public static final class SampleMessage + extends MessageMicro { + ... + } + public static final class SampleMessageContainer + extends MessageMicro { + ... + } + } + +As you can see the file-scope enum MessageType is +disassembled into two integer constants in the outer class. +In javamicro_out, all enums are disassembled and compiled +into integer constants in the parent scope (the containing +message's class or the file's (i.e. outer) class). -With the result will be com/example/TestMessages.java +You may prefer the file-scope messages to be saved in +separate files. You can do this by setting the option +java_multiple_files to true, in either the file like this: + + option java_multiple_files = true; + +or the command line like this: + +./protoc --javamicro_out=\ +java_multiple_files=true:\ +. src/proto/sample-message.proto + +The java_multiple_files option causes javamicro to use a +separate file for each file-scope message, which resides +directly in the Java package alongside the outer class: + +com/example/SampleMessageProtos.java: + + package com.example; + public final class SampleMessageProtos { + public static final int SAMPLE = 1; + public static final int EXAMPLE = 2; + } + +com/example/SampleMessage.java: + + package com.example; + public final class SampleMessage + extends MessageMicro { + ... + } + +com/example/SampleMessageContainer.java: + + package com.example; + public final class SampleMessageContainer + extends MessageMicro { + ... + } + +As you can see, the outer class now contains only the +integer constants, generated from the file-scope enum +"MessageType". Please note that message-scope enums are +still generated as integer constants in the message class. Nano version @@ -303,9 +448,19 @@ empty default. Nano Generator options -java_nano_generate_has: +java_package -> | +java_outer_classname -> | +java_multiple_files -> true or false +java_nano_generate_has -> true or false + +java_package: +java_outer_classname: +java_multiple_files: + Same as Micro version. + +java_nano_generate_has={true,false} (default: false) If true, generates a public boolean variable has - accompanying the optional or required field (not present for + accompanying each optional or required field (not present for repeated fields, groups or messages). It is set to false initially and upon clear(). If parseFrom(...) reads the field from the wire, it is set to true. This is a way for clients to inspect the "has" @@ -324,7 +479,10 @@ To use nano protobufs: java/target/protobuf-java-2.3.0-nano.jar. - Invoke with --javanano_out, e.g.: -../src/protoc '--javanano_out=java_package=src/test/proto/simple-data.proto|my_package,java_outer_classname=src/test/proto/simple-data.proto|OuterName:.' src/test/proto/simple-data.proto +./protoc '--javanano_out=\ +java_package=src/proto/simple-data.proto|my_package,\ +java_outer_classname=src/proto/simple-data.proto|OuterName:\ +.' src/proto/simple-data.proto Contributing to nano: diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java index 23768b78aa..b00f289223 100644 --- a/java/src/test/java/com/google/protobuf/NanoTest.java +++ b/java/src/test/java/com/google/protobuf/NanoTest.java @@ -33,16 +33,22 @@ package com.google.protobuf; import com.google.protobuf.nano.CodedInputByteBufferNano; import com.google.protobuf.nano.Extensions; import com.google.protobuf.nano.Extensions.AnotherMessage; +import com.google.protobuf.nano.FileScopeEnumRefNano; import com.google.protobuf.nano.InternalNano; import com.google.protobuf.nano.MessageNano; +import com.google.protobuf.nano.MessageScopeEnumRefNano; import com.google.protobuf.nano.MultipleImportingNonMultipleNano1; import com.google.protobuf.nano.MultipleImportingNonMultipleNano2; +import com.google.protobuf.nano.MultipleNameClashNano; import com.google.protobuf.nano.NanoHasOuterClass.TestAllTypesNanoHas; import com.google.protobuf.nano.NanoOuterClass; import com.google.protobuf.nano.NanoOuterClass.TestAllTypesNano; -import com.google.protobuf.nano.RecursiveMessageNano; -import com.google.protobuf.nano.SimpleMessageNano; import com.google.protobuf.nano.UnittestImportNano; +import com.google.protobuf.nano.UnittestMultipleNano; +import com.google.protobuf.nano.UnittestRecursiveNano.RecursiveMessageNano; +import com.google.protobuf.nano.UnittestSimpleNano.SimpleMessageNano; +import com.google.protobuf.nano.UnittestSingleNano.SingleMessageNano; +import com.google.protobuf.nano.UnittestStringutf8Nano.StringUtf8; import junit.framework.TestCase; @@ -2061,6 +2067,41 @@ public class NanoTest extends TestCase { assertEquals(nestedMsg2.bb, newMsg.repeatedNestedMessage[2].bb); } + /** + * Tests that code generation correctly wraps a single message into its outer + * class. The class {@code SingleMessageNano} is imported from the outer + * class {@code UnittestSingleNano}, whose name is implicit. Any error would + * cause this method to fail compilation. + */ + public void testNanoSingle() throws Exception { + SingleMessageNano msg = new SingleMessageNano(); + } + + /** + * Tests that code generation correctly skips generating the outer class if + * unnecessary, letting a file-scope entity have the same name. The class + * {@code MultipleNameClashNano} shares the same name with the file's outer + * class defined explicitly, but the file contains no other entities and has + * java_multiple_files set. Any error would cause this method to fail + * compilation. + */ + public void testNanoMultipleNameClash() throws Exception { + MultipleNameClashNano msg = new MultipleNameClashNano(); + msg.field = 0; + } + + /** + * Tests that code generation correctly handles enums in different scopes in + * a source file with the option java_multiple_files set to true. Any error + * would cause this method to fail compilation. + */ + public void testNanoMultipleEnumScoping() throws Exception { + FileScopeEnumRefNano msg1 = new FileScopeEnumRefNano(); + msg1.enumField = UnittestMultipleNano.ONE; + MessageScopeEnumRefNano msg2 = new MessageScopeEnumRefNano(); + msg2.enumField = MessageScopeEnumRefNano.TWO; + } + /** * Tests that code generation with mixed values of the java_multiple_files * options between the main source file and the imported source files would diff --git a/src/google/protobuf/compiler/javanano/javanano_enum.cc b/src/google/protobuf/compiler/javanano/javanano_enum.cc index 973ef6810d..634943b6ee 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum.cc +++ b/src/google/protobuf/compiler/javanano/javanano_enum.cc @@ -69,18 +69,6 @@ EnumGenerator::~EnumGenerator() {} void EnumGenerator::Generate(io::Printer* printer) { printer->Print("// enum $classname$\n", "classname", descriptor_->name()); - const string& file_name = descriptor_->file()->name(); - bool is_own_file = params_.java_multiple_files(file_name) || - ((descriptor_->containing_type() == NULL) && - !params_.has_java_outer_classname(file_name)); - - if (is_own_file) { - printer->Print("public final class $classname$ {\n", "classname", - descriptor_->name()); - printer->Indent(); - printer->Print("private $classname$() {}\n", "classname", - descriptor_->name()); - } for (int i = 0; i < canonical_values_.size(); i++) { map vars; vars["name"] = RenameJavaKeywords(canonical_values_[i]->name()); @@ -99,10 +87,6 @@ void EnumGenerator::Generate(io::Printer* printer) { "public static final int $name$ = $canonical_name$;\n"); } - if (is_own_file) { - printer->Outdent(); - printer->Print("}"); - } printer->Print("\n"); } diff --git a/src/google/protobuf/compiler/javanano/javanano_file.cc b/src/google/protobuf/compiler/javanano/javanano_file.cc index 711369931f..1a7b2a74d0 100644 --- a/src/google/protobuf/compiler/javanano/javanano_file.cc +++ b/src/google/protobuf/compiler/javanano/javanano_file.cc @@ -32,6 +32,8 @@ // Based on original Protocol Buffers design by // Sanjay Ghemawat, Jeff Dean, and others. +#include + #include #include #include @@ -103,56 +105,48 @@ bool FileGenerator::Validate(string* error) { return false; } - // If there is no outer class name then there must be only - // message and no enums defined in the file scope. - if (!params_.has_java_outer_classname(file_->name())) { - if (file_->message_type_count() != 1) { - error->assign(file_->name()); - error->append( - ": Java NANO_RUNTIME may only have 1 message if there is no 'option java_outer_classname'\""); - return false; - } + if (file_->service_count() != 0) { + error->assign(file_->name()); + error->append( + ": Java NANO_RUNTIME does not support services\""); + return false; + } - if (file_->enum_type_count() != 0) { - error->assign(file_->name()); - error->append( - ": Java NANO_RUNTIME must have an 'option java_outer_classname' if file scope enums are present\""); - return false; - } + if (!IsOuterClassNeeded(params_, file_)) { + return true; + } + + // Check whether legacy javanano generator would omit the outer class. + if (!params_.has_java_outer_classname(file_->name()) + && file_->message_type_count() == 1 + && file_->enum_type_count() == 0 && file_->extension_count() == 0) { + cout << "INFO: " << file_->name() << ":" << endl; + cout << "Javanano generator has changed to align with java generator. " + "An outer class will be created for this file and the single message " + "in the file will become a nested class. Use java_multiple_files to " + "skip generating the outer class, or set an explicit " + "java_outer_classname to suppress this message." << endl; } // Check that no class name matches the file's class name. This is a common // problem that leads to Java compile errors that can be hard to understand. // It's especially bad when using the java_multiple_files, since we would // end up overwriting the outer class with one of the inner ones. - int found_fileName = 0; - for (int i = 0; i < file_->enum_type_count(); i++) { - if (file_->enum_type(i)->name() == classname_) { - found_fileName += 1; - } - } - for (int i = 0; i < file_->message_type_count(); i++) { + bool found_conflict = false; + for (int i = 0; !found_conflict && i < file_->message_type_count(); i++) { if (file_->message_type(i)->name() == classname_) { - found_fileName += 1; + found_conflict = true; } } - if (file_->service_count() != 0) { + if (found_conflict) { error->assign(file_->name()); error->append( - ": Java NANO_RUNTIME does not support services\""); - return false; - } - - if (found_fileName > 1) { - error->assign(file_->name()); - error->append( - ": Cannot generate Java output because there is more than one class name, \""); + ": Cannot generate Java output because the file's outer class name, \""); error->append(classname_); error->append( "\", matches the name of one of the types declared inside it. " "Please either rename the type or use the java_outer_classname " - "option to specify a different outer class name for the .proto file." - " -- FIX THIS MESSAGE"); + "option to specify a different outer class name for the .proto file."); return false; } return true; @@ -171,13 +165,11 @@ void FileGenerator::Generate(io::Printer* printer) { "package", java_package_); } - if (params_.has_java_outer_classname(file_->name())) { - printer->Print( - "public final class $classname$ {\n" - " private $classname$() {}\n", - "classname", classname_); - printer->Indent(); - } + printer->Print( + "public final class $classname$ {\n" + " private $classname$() {}\n", + "classname", classname_); + printer->Indent(); // ----------------------------------------------------------------- @@ -186,10 +178,13 @@ void FileGenerator::Generate(io::Printer* printer) { ExtensionGenerator(file_->extension(i), params_).Generate(printer); } + // Enums. + for (int i = 0; i < file_->enum_type_count(); i++) { + EnumGenerator(file_->enum_type(i), params_).Generate(printer); + } + + // Messages. if (!params_.java_multiple_files(file_->name())) { - for (int i = 0; i < file_->enum_type_count(); i++) { - EnumGenerator(file_->enum_type(i), params_).Generate(printer); - } for (int i = 0; i < file_->message_type_count(); i++) { MessageGenerator(file_->message_type(i), params_).Generate(printer); } @@ -201,11 +196,9 @@ void FileGenerator::Generate(io::Printer* printer) { MessageGenerator(file_->message_type(i), params_).GenerateStaticVariables(printer); } - if (params_.has_java_outer_classname(file_->name())) { - printer->Outdent(); - printer->Print( - "}\n"); - } + printer->Outdent(); + printer->Print( + "}\n"); } template @@ -239,11 +232,6 @@ void FileGenerator::GenerateSiblings(const string& package_dir, OutputDirectory* output_directory, vector* file_list) { if (params_.java_multiple_files(file_->name())) { - for (int i = 0; i < file_->enum_type_count(); i++) { - GenerateSibling(package_dir, java_package_, - file_->enum_type(i), - output_directory, file_list, params_); - } for (int i = 0; i < file_->message_type_count(); i++) { GenerateSibling(package_dir, java_package_, file_->message_type(i), diff --git a/src/google/protobuf/compiler/javanano/javanano_generator.cc b/src/google/protobuf/compiler/javanano/javanano_generator.cc index 76e7263d53..5bed1b190c 100644 --- a/src/google/protobuf/compiler/javanano/javanano_generator.cc +++ b/src/google/protobuf/compiler/javanano/javanano_generator.cc @@ -138,16 +138,18 @@ bool JavaNanoGenerator::Generate(const FileDescriptor* file, vector all_files; - string java_filename = package_dir; - java_filename += file_generator.classname(); - java_filename += ".java"; - all_files.push_back(java_filename); - - // Generate main java file. - scoped_ptr output( - output_directory->Open(java_filename)); - io::Printer printer(output.get(), '$'); - file_generator.Generate(&printer); + if (IsOuterClassNeeded(params, file)) { + string java_filename = package_dir; + java_filename += file_generator.classname(); + java_filename += ".java"; + all_files.push_back(java_filename); + + // Generate main java file. + scoped_ptr output( + output_directory->Open(java_filename)); + io::Printer printer(output.get(), '$'); + file_generator.Generate(&printer); + } // Generate sibling files. file_generator.GenerateSiblings(package_dir, output_directory, &all_files); diff --git a/src/google/protobuf/compiler/javanano/javanano_helpers.cc b/src/google/protobuf/compiler/javanano/javanano_helpers.cc index 525b9dd9cb..22b4302fcd 100644 --- a/src/google/protobuf/compiler/javanano/javanano_helpers.cc +++ b/src/google/protobuf/compiler/javanano/javanano_helpers.cc @@ -167,31 +167,20 @@ string StripProto(const string& filename) { } string FileClassName(const Params& params, const FileDescriptor* file) { - string name; - if (params.has_java_outer_classname(file->name())) { - name = params.java_outer_classname(file->name()); + return params.java_outer_classname(file->name()); } else { - if ((file->message_type_count() == 1) - || (file->enum_type_count() == 0)) { - // If no outer calls and only one message then - // use the message name as the file name - name = file->message_type(0)->name(); + // Use the filename itself with underscores removed + // and a CamelCase style name. + string basename; + string::size_type last_slash = file->name().find_last_of('/'); + if (last_slash == string::npos) { + basename = file->name(); } else { - // Use the filename it self with underscores removed - // and a CamelCase style name. - string basename; - string::size_type last_slash = file->name().find_last_of('/'); - if (last_slash == string::npos) { - basename = file->name(); - } else { - basename = file->name().substr(last_slash + 1); - } - name = UnderscoresToCamelCaseImpl(StripProto(basename), true); + basename = file->name().substr(last_slash + 1); } + return UnderscoresToCamelCaseImpl(StripProto(basename), true); } - - return name; } string FileJavaPackage(const Params& params, const FileDescriptor* file) { @@ -207,38 +196,27 @@ string FileJavaPackage(const Params& params, const FileDescriptor* file) { } } -string ToJavaName(const Params& params, const string& full_name, - const FileDescriptor* file) { - string result; - if (params.java_multiple_files(file->name())) { - result = FileJavaPackage(params, file); - } else { - result = ClassName(params, file); +bool IsOuterClassNeeded(const Params& params, const FileDescriptor* file) { + // Enums and extensions need the outer class as the scope. + if (file->enum_type_count() != 0 || file->extension_count() != 0) { + return true; } - if (file->package().empty()) { - result += '.'; - result += full_name; + // Messages need the outer class only if java_multiple_files is false. + return !params.java_multiple_files(file->name()); +} + +string ToJavaName(const Params& params, const string& name, bool is_class, + const Descriptor* parent, const FileDescriptor* file) { + string result; + if (parent != NULL) { + result.append(ClassName(params, parent)); + } else if (is_class && params.java_multiple_files(file->name())) { + result.append(FileJavaPackage(params, file)); } else { - // Strip the proto package from full_name since we've replaced it with - // the Java package. If there isn't an outer classname then strip it too. - int sizeToSkipPackageName = file->package().size(); - int sizeToSkipOutClassName; - if (params.has_java_outer_classname(file->name())) { - sizeToSkipOutClassName = 0; - } else { - sizeToSkipOutClassName = - full_name.find_first_of('.', sizeToSkipPackageName + 1); - } - int sizeToSkip = sizeToSkipOutClassName > 0 ? - sizeToSkipOutClassName : sizeToSkipPackageName; - string class_name = full_name.substr(sizeToSkip + 1); - if (class_name == FileClassName(params, file)) { - // Done class_name is already present. - } else { - result += '.'; - result += class_name; - } + result.append(ClassName(params, file)); } + if (!result.empty()) result.append(1, '.'); + result.append(RenameJavaKeywords(name)); return result; } @@ -250,61 +228,14 @@ string ClassName(const Params& params, const FileDescriptor* descriptor) { } string ClassName(const Params& params, const EnumDescriptor* descriptor) { - string result; - const FileDescriptor* file = descriptor->file(); - const string file_name = file->name(); - const string full_name = descriptor->full_name(); - - // Remove enum class name as we use int's for enums - int last_dot_in_name = full_name.find_last_of('.'); - string base_name = full_name.substr(0, last_dot_in_name); - - if (!file->package().empty()) { - if (file->package() == base_name.substr(0, file->package().size())) { - // Remove package name leaving just the parent class of the enum - int offset = file->package().size(); - if (base_name.size() > offset) { - // Remove period between package and class name if there is a classname - offset += 1; - } - base_name = base_name.substr(offset); - } else { - GOOGLE_LOG(FATAL) << "Expected package name to start an enum"; - } - } - - // Construct the path name from the package and outer class - - // Add the java package name if it exists - if (params.has_java_package(file_name)) { - result += params.java_package(file_name); - } - - // If the java_multiple_files option is present, we will generate enums into separate - // classes, each named after the original enum type. This takes precedence over - // any outer_classname. - if (params.java_multiple_files(file_name) && last_dot_in_name != string::npos) { - string enum_simple_name = full_name.substr(last_dot_in_name + 1); - if (!result.empty()) { - result += "."; - } - result += enum_simple_name; - } else if (params.has_java_outer_classname(file_name)) { - // Add the outer classname if it exists - if (!result.empty()) { - result += "."; - } - result += params.java_outer_classname(file_name); - } - - // Create the full class name from the base and path - if (!base_name.empty()) { - if (!result.empty()) { - result += "."; - } - result += base_name; + // An enum's class name is the enclosing message's class name or the outer + // class name. + const Descriptor* parent = descriptor->containing_type(); + if (parent != NULL) { + return ClassName(params, parent); + } else { + return ClassName(params, descriptor->file()); } - return result; } string FieldConstantName(const FieldDescriptor *field) { diff --git a/src/google/protobuf/compiler/javanano/javanano_helpers.h b/src/google/protobuf/compiler/javanano/javanano_helpers.h index afadf60848..430969bf90 100644 --- a/src/google/protobuf/compiler/javanano/javanano_helpers.h +++ b/src/google/protobuf/compiler/javanano/javanano_helpers.h @@ -73,25 +73,35 @@ string FileClassName(const Params& params, const FileDescriptor* file); // Returns the file's Java package name. string FileJavaPackage(const Params& params, const FileDescriptor* file); -// Converts the given fully-qualified name in the proto namespace to its -// fully-qualified name in the Java namespace, given that it is in the given -// file. -string ToJavaName(const Params& params, const string& full_name, - const FileDescriptor* file); +// Returns whether the Java outer class is needed, i.e. whether the option +// java_multiple_files is false, or the proto file contains any file-scope +// enums/extensions. +bool IsOuterClassNeeded(const Params& params, const FileDescriptor* file); + +// Converts the given simple name of a proto entity to its fully-qualified name +// in the Java namespace, given that it is in the given file enclosed in the +// given parent message (or NULL for file-scope entities). Whether the file's +// outer class name should be included in the return value depends on factors +// inferrable from the given arguments, including is_class which indicates +// whether the entity translates to a Java class. +string ToJavaName(const Params& params, const string& name, bool is_class, + const Descriptor* parent, const FileDescriptor* file); // These return the fully-qualified class name corresponding to the given // descriptor. inline string ClassName(const Params& params, const Descriptor* descriptor) { - return ToJavaName(params, descriptor->full_name(), descriptor->file()); + return ToJavaName(params, descriptor->name(), true, + descriptor->containing_type(), descriptor->file()); } string ClassName(const Params& params, const EnumDescriptor* descriptor); inline string ClassName(const Params& params, const ServiceDescriptor* descriptor) { - return ToJavaName(params, descriptor->full_name(), descriptor->file()); + return ToJavaName(params, descriptor->name(), true, NULL, descriptor->file()); } inline string ExtensionIdentifierName(const Params& params, const FieldDescriptor* descriptor) { - return ToJavaName(params, descriptor->full_name(), descriptor->file()); + return ToJavaName(params, descriptor->name(), false, + descriptor->extension_scope(), descriptor->file()); } string ClassName(const Params& params, const FileDescriptor* descriptor); diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index de56944136..851c4434c0 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -114,7 +114,7 @@ void MessageGenerator::GenerateStaticVariableInitializers( io::Printer* printer) { // Generate static member initializers for all nested types. for (int i = 0; i < descriptor_->nested_type_count(); i++) { - // TODO(kenton): Reuse MessageGenerator objects? + // TODO(kenton): Reuse MessageGenerator objects? MessageGenerator(descriptor_->nested_type(i), params_) .GenerateStaticVariableInitializers(printer); } @@ -124,15 +124,7 @@ void MessageGenerator::Generate(io::Printer* printer) { const string& file_name = descriptor_->file()->name(); bool is_own_file = params_.java_multiple_files(file_name) - || ((descriptor_->containing_type() == NULL) - && !params_.has_java_outer_classname(file_name)); - -#if 0 - GOOGLE_LOG(INFO) << "is_own_file=" << is_own_file; - GOOGLE_LOG(INFO) << "containing_type()=" << ((descriptor_->containing_type() == NULL) ? "NULL" : "not null"); - GOOGLE_LOG(INFO) << "java_multiple_files()=" << params_.java_multiple_files(); - GOOGLE_LOG(INFO) << "has_java_outer_classname()=" << params_.has_java_outer_classname(file_->name()); -#endif + && descriptor_->containing_type() == NULL; if (!params_.store_unknown_fields() && (descriptor_->extension_count() != 0 || descriptor_->extension_range_count() != 0)) { @@ -355,25 +347,21 @@ void MessageGenerator::GenerateMergeFromMethods(io::Printer* printer) { void MessageGenerator:: GenerateParseFromMethods(io::Printer* printer) { - bool is_own_file = - descriptor_->containing_type() == NULL; - // Note: These are separate from GenerateMessageSerializationMethods() // because they need to be generated even for messages that are optimized // for code size. printer->Print( - "public $static$ $classname$ parseFrom(byte[] data)\n" + "public static $classname$ parseFrom(byte[] data)\n" " throws com.google.protobuf.nano.InvalidProtocolBufferNanoException {\n" " return com.google.protobuf.nano.MessageNano.mergeFrom(new $classname$(), data);\n" "}\n" "\n" - "public $static$ $classname$ parseFrom(\n" + "public static $classname$ parseFrom(\n" " com.google.protobuf.nano.CodedInputByteBufferNano input)\n" " throws java.io.IOException {\n" " return new $classname$().mergeFrom(input);\n" "}\n" "\n", - "static", (is_own_file ? "static" : ""), "classname", descriptor_->name()); } diff --git a/src/google/protobuf/unittest_enum_multiplejava_nano.proto b/src/google/protobuf/unittest_multiple_nameclash_nano.proto similarity index 82% rename from src/google/protobuf/unittest_enum_multiplejava_nano.proto rename to src/google/protobuf/unittest_multiple_nameclash_nano.proto index adf017d502..f2f62c4e7a 100644 --- a/src/google/protobuf/unittest_enum_multiplejava_nano.proto +++ b/src/google/protobuf/unittest_multiple_nameclash_nano.proto @@ -28,26 +28,14 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -// Author: bduff@google.com (Brian Duff) -// +// Author: maxtroy@google.com (Max Cai) package protobuf_unittest_import; option java_package = "com.google.protobuf.nano"; -option java_outer_classname = "NanoEnumsWithMultipleFiles"; +option java_outer_classname = "MultipleNameClashNano"; option java_multiple_files = true; -enum FirstTopLevelEnum { - FIRST_TOP_LEVEL_FIRST = 1; - FIRST_TOP_LEVEL_SECOND = 2; -} - -enum SecondTopLevelEnum { - SECOND_TOP_LEVEL_FIRST = 1; - SECOND_TOP_LEVEL_SECOND = 2; -} - -message SomeMessage { - optional FirstTopLevelEnum first = 1; - optional SecondTopLevelEnum second = 2; +message MultipleNameClashNano { + optional int32 field = 1; } diff --git a/src/google/protobuf/unittest_multiple_nano.proto b/src/google/protobuf/unittest_multiple_nano.proto index bd174c270b..5ae790b673 100644 --- a/src/google/protobuf/unittest_multiple_nano.proto +++ b/src/google/protobuf/unittest_multiple_nano.proto @@ -35,9 +35,25 @@ package protobuf_unittest_import; import "google/protobuf/unittest_import_nano.proto"; option java_package = "com.google.protobuf.nano"; -option java_outer_classname = "NanoMultipleImportingNonMultiple"; option java_multiple_files = true; +enum FileScopeEnum { + ONE = 1; + TWO = 2; +} + +message FileScopeEnumRefNano { + optional FileScopeEnum enum_field = 1; +} + +message MessageScopeEnumRefNano { + enum MessageScopeEnum { + ONE = 1; + TWO = 2; + } + optional MessageScopeEnum enum_field = 1; +} + message MultipleImportingNonMultipleNano1 { optional ImportMessageNano field = 1; } diff --git a/src/google/protobuf/unittest_recursive_nano.proto b/src/google/protobuf/unittest_recursive_nano.proto index a62613ed35..3d3a6aa4b9 100644 --- a/src/google/protobuf/unittest_recursive_nano.proto +++ b/src/google/protobuf/unittest_recursive_nano.proto @@ -34,6 +34,8 @@ package protobuf_unittest_import; option java_package = "com.google.protobuf.nano"; +// Explicit outer classname to suppress legacy info. +option java_outer_classname = "UnittestRecursiveNano"; message RecursiveMessageNano { message NestedMessage { diff --git a/src/google/protobuf/unittest_simple_nano.proto b/src/google/protobuf/unittest_simple_nano.proto index 287e962f84..0c780c7252 100644 --- a/src/google/protobuf/unittest_simple_nano.proto +++ b/src/google/protobuf/unittest_simple_nano.proto @@ -34,6 +34,8 @@ package protobuf_unittest_import; option java_package = "com.google.protobuf.nano"; +// Explicit outer classname to suppress legacy info. +option java_outer_classname = "UnittestSimpleNano"; message SimpleMessageNano { message NestedMessage { diff --git a/src/google/protobuf/unittest_single_nano.proto b/src/google/protobuf/unittest_single_nano.proto new file mode 100644 index 0000000000..fcb1539d47 --- /dev/null +++ b/src/google/protobuf/unittest_single_nano.proto @@ -0,0 +1,38 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// http://code.google.com/p/protobuf/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Author: maxtroy@google.com (Max Cai) + +package protobuf_unittest_import; + +option java_package = "com.google.protobuf.nano"; + +message SingleMessageNano { +} diff --git a/src/google/protobuf/unittest_stringutf8_nano.proto b/src/google/protobuf/unittest_stringutf8_nano.proto index da624b0e59..2bbf2b7b37 100644 --- a/src/google/protobuf/unittest_stringutf8_nano.proto +++ b/src/google/protobuf/unittest_stringutf8_nano.proto @@ -34,6 +34,8 @@ package protobuf_unittest_import; option java_package = "com.google.protobuf.nano"; +// Explicit outer classname to suppress legacy info. +option java_outer_classname = "UnittestStringutf8Nano"; message StringUtf8 { optional string id = 1;