Yang code generator cleanup 70/25570/5
authorStephen Kitt <skitt@redhat.com>
Fri, 21 Aug 2015 11:26:10 +0000 (13:26 +0200)
committerGerrit Code Review <gerrit@opendaylight.org>
Fri, 18 Sep 2015 09:27:08 +0000 (09:27 +0000)
Two axes:
* make the generated code slightly better
* make the generator code slightly better

Fixes in generator code:
* fields which can be local variables
* map overwrite detection using put()'s return value
* logging with exceptions
* using File's parent/child handling instead of concatenating strings
* if (condition) { return true; } else { return false; }
  simplification
* if (... == false) -> if (!...)
* interface method declarations are public by default
* Collections.singletonList() instead of Arrays.asList() for
  single-item collections
* StringBuilder instead of StringBuffer
* String concatenation instead of straight-forward StringBuilder
  (i.e. non-conditional append() calls)
* typos: hierarchchical -> hierarchical, intends -> indents
* drop unused parameter from
  AbstractAttributesProcessor::processAttributes()
* when processing only values in a map, use .values() rather than
  .entrySet()
* split StringBuilder::append(a + b) into two ::append() calls
* clean up the method parameter construction in MethodSerializer (to
  avoid deleting the ", " at the end)

Fixes in generated code:
* use { } as requested the Checkstyle rules
* handle empty service interface sets explicitly

It would be nice to use the <> operator without specifying the type
where possible, but the Eclipse AST parser used in the unit tests is
too old for this (so the generated code is correct and works
elsewhere, but fails the unit tests).

Change-Id: I725ce8d98f9ee389d394772733663ecacaccf8fa
Signed-off-by: Stephen Kitt <skitt@redhat.com>
15 files changed:
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/JMXGenerator.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/FtlTemplate.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/RuntimeRegistratorFtlTemplate.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/TemplateFactory.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/TypeHelper.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/model/ConstructorSerializer.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/model/FieldSerializer.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/model/HeaderSerializer.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/model/MethodSerializer.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/model/ModuleFieldSerializer.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/gofactory/AbsFactoryGeneratedObjectFactory.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/gofactory/AbsModuleGeneratedObjectFactory.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/gofactory/ConcreteModuleGeneratedObjectFactory.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/java/GeneratedObjectBuilder.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/util/StringUtil.java

index 4ad8fd7..2216f19 100644 (file)
@@ -65,7 +65,6 @@ public class JMXGenerator implements BasicCodeGenerator, MavenProjectAware {
     private static final Logger LOG = LoggerFactory.getLogger(JMXGenerator.class);
     private static final Pattern NAMESPACE_MAPPING_PATTERN = Pattern.compile("(.+)" + NAMESPACE_TO_PACKAGE_DIVIDER + "(.+)");
 
-    private PackageTranslator packageTranslator;
     private final CodeWriter codeWriter;
     private Map<String, String> namespaceToPackageMapping;
     private File resourceBaseDir;
@@ -92,7 +91,7 @@ public class JMXGenerator implements BasicCodeGenerator, MavenProjectAware {
                 .checkArgument(namespaceToPackageMapping != null && !namespaceToPackageMapping.isEmpty(),
                         "No namespace to package mapping provided in additionalConfiguration");
 
-        packageTranslator = new PackageTranslator(namespaceToPackageMapping);
+        PackageTranslator packageTranslator = new PackageTranslator(namespaceToPackageMapping);
 
         if (!outputBaseDir.exists()) {
             outputBaseDir.mkdirs();
@@ -112,11 +111,9 @@ public class JMXGenerator implements BasicCodeGenerator, MavenProjectAware {
             for (Entry<QName, ServiceInterfaceEntry> sieEntry : namesToSIEntries
                     .entrySet()) {
                 // merge value into qNamesToSIEs
-                if (qNamesToSIEs.containsKey(sieEntry.getKey()) == false) {
-                    qNamesToSIEs.put(sieEntry.getKey(), sieEntry.getValue());
-                } else {
+                if (qNamesToSIEs.put(sieEntry.getKey(), sieEntry.getValue()) != null) {
                     throw new IllegalStateException(
-                        "Cannot add two SIE  with same qname "
+                        "Cannot add two SIE with same qname "
                                 + sieEntry.getValue());
                 }
             }
@@ -177,7 +174,7 @@ public class JMXGenerator implements BasicCodeGenerator, MavenProjectAware {
                 Files.write(fullyQualifiedNamesOfFactories.toString(), serviceLoaderFile, StandardCharsets.UTF_8);
             } catch (IOException e) {
                 String message = "Cannot write to " + serviceLoaderFile;
-                LOG.error(message);
+                LOG.error(message, e);
                 throw new RuntimeException(message, e);
             }
         }
@@ -186,12 +183,11 @@ public class JMXGenerator implements BasicCodeGenerator, MavenProjectAware {
 
     @VisibleForTesting
     static File concatFolders(final File projectBaseDir, final String... folderNames) {
-        StringBuilder b = new StringBuilder();
-        for (String folder : folderNames) {
-            b.append(folder);
-            b.append(File.separator);
+        File result = projectBaseDir;
+        for (String folder: folderNames) {
+            result = new File(result, folder);
         }
-        return new File(projectBaseDir, b.toString());
+        return result;
     }
 
     @Override
@@ -204,13 +200,7 @@ public class JMXGenerator implements BasicCodeGenerator, MavenProjectAware {
     private boolean extractModuleFactoryBoolean(
             final Map<String, String> additionalCfg) {
         String bool = additionalCfg.get(MODULE_FACTORY_FILE_BOOLEAN);
-        if (bool == null) {
-            return true;
-        }
-        if ("false".equals(bool)) {
-            return false;
-        }
-        return true;
+        return !"false".equals(bool);
     }
 
     private static Map<String, String> extractNamespaceMapping(
@@ -254,11 +244,11 @@ public class JMXGenerator implements BasicCodeGenerator, MavenProjectAware {
             if (files.contains(file)) {
                 List<File> undeletedFiles = Lists.newArrayList();
                 for (File presentFile : files) {
-                    if (presentFile.delete() == false) {
+                    if (!presentFile.delete()) {
                         undeletedFiles.add(presentFile);
                     }
                 }
-                if (undeletedFiles.isEmpty() == false) {
+                if (!undeletedFiles.isEmpty()) {
                     LOG.error(
                             "Illegal state occurred: Unable to delete already generated files, undeleted files: {}",
                             undeletedFiles);
index 2e867d3..68d0b71 100644 (file)
@@ -25,13 +25,13 @@ public interface FtlTemplate {
 
     Optional<String> getMaybeJavadoc();
 
-    public List<Annotation> getAnnotations();
+    List<Annotation> getAnnotations();
 
     TypeDeclaration getTypeDeclaration();
 
-    public String getFullyQualifiedName();
+    String getFullyQualifiedName();
 
-    public List<Field> getFields();
+    List<Field> getFields();
 
     List<? extends Method> getMethods();
 
index 13345d6..27fb2bc 100644 (file)
@@ -15,7 +15,6 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
 import java.io.Closeable;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -41,8 +40,8 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
             String name, List<Field> fields, List<MethodDefinition> methods) {
         // TODO header
         super(null, runtimeBeanEntry.getPackageName(), name, Collections
-                .<String> emptyList(), Arrays.asList(Closeable.class
-                    .getCanonicalName()), fields, methods);
+                .<String> emptyList(), Collections.singletonList(Closeable.class
+                .getCanonicalName()), fields, methods);
     }
 
     public static RuntimeBeanEntry findRoot(
@@ -65,7 +64,7 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
 
     private static String constructConstructorBody(
             List<Field> constructorParameters) {
-        StringBuffer constructorBody = new StringBuffer();
+        StringBuilder constructorBody = new StringBuilder();
         for (Field field : constructorParameters) {
             constructorBody.append("this.");
             constructorBody.append(field.getName());
@@ -109,16 +108,16 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
             StringBuilder registerBody = new StringBuilder();
             registerBody.append(format("%s %s = this.%s.registerRoot(%s);\n",
                     HierarchicalRuntimeBeanRegistration.class
-                            .getCanonicalName(), hierachchicalRegistration
+                            .getCanonicalName(), hierachicalRegistration
                             .getName(), rootRuntimeBeanRegistratorField
                             .getName(), rbParameter.getName()));
             registerBody.append(format("return new %s(%s);\n",
                     rootFtlFile.getFullyQualifiedName(),
-                    hierachchicalRegistration.getName()));
+                    hierachicalRegistration.getName()));
 
             MethodDefinition registerMethod = new MethodDefinition(
                     childRegistratorFQN, "register",
-                    Arrays.asList(rbParameter), registerBody.toString());
+                    Collections.singletonList(rbParameter), registerBody.toString());
             methods.add(registerMethod);
         }
 
@@ -128,11 +127,11 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
         // TODO add header
         GeneralClassTemplate registrator = new GeneralClassTemplate(null,
                 rootRB.getPackageName(), registratorName,
-                Collections.<String> emptyList(), Arrays.asList(Closeable.class
-                        .getCanonicalName()), constructorParameters, methods);
+                Collections.<String> emptyList(), Collections.singletonList(Closeable.class
+                .getCanonicalName()), constructorParameters, methods);
 
-        checkState(RuntimeRegistratorFtlTemplates.containsKey(registrator
-                .getTypeDeclaration().getName()) == false, "Name conflict: "
+        checkState(!RuntimeRegistratorFtlTemplates.containsKey(registrator
+                .getTypeDeclaration().getName()), "Name conflict: "
                 + registrator.getTypeDeclaration().getName());
         Map<String, FtlTemplate> result = new HashMap<>();
         result.putAll(RuntimeRegistratorFtlTemplates);
@@ -140,7 +139,7 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
         return result;
     }
 
-    private static Field hierachchicalRegistration = new Field(
+    private static Field hierachicalRegistration = new Field(
             Lists.newArrayList("final"),
             HierarchicalRuntimeBeanRegistration.class.getCanonicalName(),
             "registration");
@@ -173,7 +172,7 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
         Set<String> currentOccupiedKeys = new HashSet<>(occupiedKeys);
         currentOccupiedKeys.add(parent.getJavaNamePrefix());
 
-        Field registratorsMapField = new Field(Arrays.asList("final"),
+        Field registratorsMapField = new Field(Collections.singletonList("final"),
                 TypeHelper.getGenericType(Map.class, String.class,
                         AtomicInteger.class), "unkeyedMap", "new "
                         + TypeHelper.getGenericType(HashMap.class,
@@ -223,7 +222,7 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
                 body.append(format("String value = %s;\n", value));
                 body.append(format("%s r = %s.register(key, value, bean);\n",
                         HierarchicalRuntimeBeanRegistration.class
-                                .getCanonicalName(), hierachchicalRegistration
+                                .getCanonicalName(), hierachicalRegistration
                                 .getName()));
                 body.append(format("return new %s(r);",
                         childRegistrator.getFullyQualifiedName()));
@@ -231,9 +230,9 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
                 Field param = new Field(Lists.newArrayList("final"),
                         child.getJavaNameOfRuntimeMXBean(), "bean");
                 MethodDefinition register = new MethodDefinition(
-                        Arrays.asList("synchronized"),
+                        Collections.singletonList("synchronized"),
                         childRegistrator.getFullyQualifiedName(), "register",
-                        Arrays.asList(param), Collections.<String> emptyList(),
+                        Collections.singletonList(param), Collections.<String> emptyList(),
                         Collections.<Annotation> emptyList(), body.toString());
                 methods.add(register);
 
@@ -243,14 +242,13 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
         // create parent registration
         String createdName = getJavaNameOfRuntimeRegistration(parent.getJavaNamePrefix());
 
-        List<Field> constructorParameters = Arrays
-                .asList(hierachchicalRegistration);
+        List<Field> constructorParameters = Collections.singletonList(hierachicalRegistration);
         String constructorBody = constructConstructorBody(constructorParameters);
 
         MethodDefinition constructor = MethodDefinition.createConstructor(
                 createdName, constructorParameters, constructorBody);
 
-        MethodDefinition closeRegistrator = createCloseMethodToCloseField(hierachchicalRegistration);
+        MethodDefinition closeRegistrator = createCloseMethodToCloseField(hierachicalRegistration);
         methods.add(closeRegistrator);
         methods.add(constructor);
         List<Field> privateFields = Lists.newArrayList(registratorsMapField);
@@ -261,8 +259,8 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
 
         LinkedHashMap<String, RuntimeRegistratorFtlTemplate> result = new LinkedHashMap<>();
         result.put(created.getTypeDeclaration().getName(), created);
-        checkState(unorderedResult.containsKey(created.getTypeDeclaration()
-                .getName()) == false, "Naming conflict: "
+        checkState(!unorderedResult.containsKey(created.getTypeDeclaration()
+                .getName()), "Naming conflict: "
                 + created.getTypeDeclaration().getName());
         result.putAll(unorderedResult);
         return result;
index 1eaa35f..0030c3c 100644 (file)
@@ -11,7 +11,6 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -64,7 +63,7 @@ public class TemplateFactory {
         { // create GeneralInterfaceFtlFile for runtime MXBean. Attributes will
           // be transformed to getter methods
             String mxBeanTypeName = entry.getJavaNameOfRuntimeMXBean();
-            List<String> extendedInterfaces = Arrays.asList(RuntimeBean.class
+            List<String> extendedInterfaces = Collections.singletonList(RuntimeBean.class
                     .getCanonicalName());
             List<MethodDeclaration> methods = new ArrayList<>();
 
@@ -179,8 +178,7 @@ public class TemplateFactory {
     public static AbstractFactoryTemplate abstractFactoryTemplateFromMbe(
             final ModuleMXBeanEntry mbe) {
         AbstractFactoryAttributesProcessor attrProcessor = new AbstractFactoryAttributesProcessor();
-        attrProcessor.processAttributes(mbe.getAttributes(),
-                mbe.getPackageName());
+        attrProcessor.processAttributes(mbe.getAttributes());
 
 
 
@@ -205,7 +203,7 @@ public class TemplateFactory {
         boolean generateRuntime = false;
         String registratorFullyQualifiedName = null;
         if (mbe.getRuntimeBeans() != null
-                && mbe.getRuntimeBeans().isEmpty() == false) {
+                && !mbe.getRuntimeBeans().isEmpty()) {
             generateRuntime = true;
             RuntimeBeanEntry rootEntry = RuntimeRegistratorFtlTemplate
                     .findRoot(mbe.getRuntimeBeans());
@@ -294,7 +292,7 @@ public class TemplateFactory {
                 continue;
             }
 
-            Preconditions.checkState(yangPropertiesToTypesMap.containsKey(returnType.getAttributeYangName()) == false,
+            Preconditions.checkState(!yangPropertiesToTypesMap.containsKey(returnType.getAttributeYangName()),
                     "Duplicate TO %s for %s", returnType.getAttributeYangName(), rbe);
             yangPropertiesToTypesMap.put(returnType.getAttributeYangName(), returnType);
         }
@@ -435,8 +433,8 @@ public class TemplateFactory {
 
             private MethodDefinition getEquals(final Map<String, AttributeIfc> attrs) {
                 final StringBuilder equalsBodyBuilder = new StringBuilder(
-                        "        if (this == o) return true;\n" +
-                        "        if (o == null || getClass() != o.getClass()) return false;\n");
+                        "        if (this == o) { return true; }\n" +
+                        "        if (o == null || getClass() != o.getClass()) { return false; }\n");
                 equalsBodyBuilder.append(String.format(
                         "        final %s that = (%s) o;\n", name, name));
                 for (AttributeIfc s : attrs.values()) {
@@ -554,24 +552,19 @@ public class TemplateFactory {
 
         private final List<Field> fields = Lists.newArrayList();
 
-        void processAttributes(final Map<String, AttributeIfc> attributes,
-                final String packageName) {
-            for (Entry<String, AttributeIfc> attrEntry : attributes.entrySet()) {
-                String type;
-                String nullableDefaultWrapped = null;
-                AttributeIfc attributeIfc = attrEntry.getValue();
-
+        void processAttributes(final Map<String, AttributeIfc> attributes) {
+            for (AttributeIfc attributeIfc : attributes.values()) {
                 if (attributeIfc instanceof TypedAttribute) {
                     TypedAttribute typedAttribute = (TypedAttribute) attributeIfc;
-                    type = serializeType(typedAttribute.getType());
+                    String type = serializeType(typedAttribute.getType());
+
+                    fields.add(new Field(type, attributeIfc
+                            .getUpperCaseCammelCase(), null));
                 } else {
                     throw new UnsupportedOperationException(
                             "Attribute not supported: "
                                     + attributeIfc.getClass());
                 }
-
-                fields.add(new Field(type, attributeIfc
-                        .getUpperCaseCammelCase(), nullableDefaultWrapped));
             }
         }
 
index e097516..468d20c 100644 (file)
@@ -14,7 +14,7 @@ class TypeHelper {
      * "List<String>" for input parameters List.class, String.class
      */
     static String getGenericType(Class<?> type, Class<?>... parameters) {
-        StringBuffer sb = new StringBuffer();
+        StringBuilder sb = new StringBuilder();
         sb.append(type.getCanonicalName());
         if (parameters.length > 0) {
             sb.append("<");
index c257aa6..23d3ac2 100644 (file)
@@ -16,7 +16,7 @@ public class ConstructorSerializer {
         if (constr.isPublic()) {
             build.append("public ");
         }
-        build.append(constr.getTypeName() + " ");
+        build.append(constr.getTypeName());
         build.append("() {");
         build.append("\n");
         build.append("    ");
index 90a2b56..7105c39 100644 (file)
@@ -14,12 +14,12 @@ public class FieldSerializer {
         StringBuilder build = new StringBuilder();
         build.append("private ");
         for (String mod : field.getModifiers()) {
-            build.append(mod + " ");
+            build.append(mod).append(" ");
         }
-        build.append(field.getType() + " ");
+        build.append(field.getType()).append(" ");
         build.append(field.getName());
         if (field.getDefinition() != null) {
-            build.append(" = " + field.getDefinition());
+            build.append(" = ").append(field.getDefinition());
         }
         build.append(";");
         build.append("\n");
index 89a2cf7..5eabeab 100644 (file)
@@ -9,35 +9,22 @@
 package org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model;
 
 import java.util.Date;
+
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.JMXGenerator;
 
 public class HeaderSerializer {
     private static final String GENERATOR_CLASS = JMXGenerator.class.getCanonicalName();
 
     public static String toString(Header header) {
-        StringBuilder build = new StringBuilder();
-
-
-        build.append("Generated file");
-        build.append("\n");
-        build.append("\n");
-        build.append("Generated from: ");
-        //build.append(header.toString());
-
-        build.append("yang module name: ");
-        build.append(header.getYangModuleName());
-        build.append(" yang module local name: ");
-        build.append(header.getYangModuleLocalName());
-
-        build.append("\n");
-        build.append("Generated by: " + GENERATOR_CLASS);
-        build.append("\n");
-        build.append("Generated at: " + new Date());
-        build.append("\n");
-        build.append("\n");
-        build.append("Do not modify this file unless it is present under src/main directory ");
-
-        return build.toString();
+        return "Generated file" + "\n" +
+                "\n" +
+                "Generated from: " +
+                "yang module name: " + header.getYangModuleName() +
+                " yang module local name: " + header.getYangModuleLocalName() + "\n" +
+                "Generated by: " + GENERATOR_CLASS + "\n" +
+                "Generated at: " + new Date() + "\n" +
+                "\n" +
+                "Do not modify this file unless it is present under src/main directory ";
     }
 
 }
index 7d2b9fc..eba6e53 100644 (file)
@@ -24,42 +24,40 @@ class MethodSerializer {
 
         build.append("    " + "public ");
         for (String mod : method.getModifiers()) {
-            build.append(mod + " ");
+            build.append(mod).append(" ");
         }
-        build.append(method.getReturnType() + " ");
+        build.append(method.getReturnType()).append(" ");
 
-        build.append(method.getName() + "(");
+        build.append(method.getName()).append("(");
+        boolean firstParam = true;
         for (Field param : method.getParameters()) {
+            if (!firstParam) {
+                build.append(", ");
+            }
             for (String mod : param.getModifiers()) {
-                build.append(mod + " ");
+                build.append(mod).append(" ");
             }
-            build.append(param.getType() + " ");
-            build.append(param.getName() + ", ");
-        }
-        if (method.getParameters().isEmpty()) {
-            build.append(")");
-        } else {
-            build.deleteCharAt(build.length() - 1);
-            build.deleteCharAt(build.length() - 1);
-            build.append(')');
+            build.append(param.getType()).append(" ");
+            build.append(param.getName());
+            firstParam = false;
         }
+        build.append(")");
 
         if (method instanceof MethodDeclaration) {
             build.append(";");
             build.append("\n");
         } else if (method instanceof MethodDefinition) {
-            if (!((MethodDefinition) method).getThrowsExceptions()
-                    .isEmpty()) {
+            MethodDefinition definition = (MethodDefinition) method;
+            if (!definition.getThrowsExceptions().isEmpty()) {
                 build.append(" throws ");
             }
-            for (String ex : ((MethodDefinition) method)
-                    .getThrowsExceptions()) {
-                build.append(ex + " ");
+            for (String ex : definition.getThrowsExceptions()) {
+                build.append(ex).append(" ");
             }
             build.append(" {");
             build.append("\n");
             build.append("        ");
-            build.append(((MethodDefinition) method).getBody());
+            build.append(definition.getBody());
             build.append("\n");
             build.append("    ");
             build.append("}");
index 6211efe..abb3e4c 100644 (file)
@@ -16,28 +16,32 @@ public class ModuleFieldSerializer {
     public static String toString(ModuleField moduleField) {
         StringBuilder builder = new StringBuilder();
         builder.append("    ");
-        builder.append("public static final "
-                + JmxAttribute.class.getCanonicalName() + " "
-                + moduleField.getName() + "JmxAttribute = new "
-                + JmxAttribute.class.getCanonicalName() + "(\""
-                + moduleField.getAttributeName() + "\");");
+        builder.append("public static final ");
+        builder.append(JmxAttribute.class.getCanonicalName());
+        builder.append(" ");
+        builder.append(moduleField.getName());
+        builder.append("JmxAttribute = new ");
+        builder.append(JmxAttribute.class.getCanonicalName());
+        builder.append("(\"");
+        builder.append(moduleField.getAttributeName());
+        builder.append("\");");
         builder.append("\n");
 
         builder.append("     private ");
         for (String mod : moduleField.getModifiers()) {
-            builder.append(mod + " ");
+            builder.append(mod).append(" ");
         }
-        builder.append(moduleField.getType() + " ");
+        builder.append(moduleField.getType()).append(" ");
         builder.append(moduleField.getName());
         if (moduleField.getNullableDefault() != null) {
-            builder.append(" = " + moduleField.getNullableDefault());
+            builder.append(" = ").append(moduleField.getNullableDefault());
         }
         builder.append(";");
 
         if (moduleField.isDependent()) {
             String comment = moduleField.getDependency().isMandatory() ? "mandatory"
                     : "optional";
-            builder.append(" // " + comment);
+            builder.append(" // ").append(comment);
         }
         builder.append("\n");
 
index 8c65eaa..25314cb 100644 (file)
@@ -114,11 +114,12 @@ public class AbsFactoryGeneratedObjectFactory {
                 "throw new UnsupportedOperationException(\"Class reloading is not supported\");\n"+
             "}\n", moduleFQN, DynamicMBeanWithInstance.class.getCanonicalName()));
 
+        // TODO The generic specifier in HashSet<> isn't necessary, but the Eclipse AST parser used in the unit tests doesn't support this
         b.addToBody(format("\n"+
             "@Override\n"+
             "public java.util.Set<%s> getDefaultModules(org.opendaylight.controller.config.api.DependencyResolverFactory dependencyResolverFactory, %s bundleContext) {\n"+
-                "return new java.util.HashSet<%s>();\n"+
-            "}\n", moduleFQN, BUNDLE_CONTEXT, moduleFQN));
+                "return new java.util.HashSet<%1$s>();\n"+
+            "}\n", moduleFQN, BUNDLE_CONTEXT));
 
         return new GeneratedObjectBuilder(b.build()).toGeneratedObject();
     }
@@ -129,7 +130,7 @@ public class AbsFactoryGeneratedObjectFactory {
             format("public %s createModule(String instanceName, %s dependencyResolver, %s old, %s bundleContext) throws Exception {\n",
                                 Module.class.getCanonicalName(), DependencyResolver.class.getCanonicalName(),
                                 DynamicMBeanWithInstance.class.getCanonicalName(), BUNDLE_CONTEXT)+
-                format("%s oldModule = null;\n",moduleFQN)+
+                format("%s oldModule;\n",moduleFQN)+
                 "try {\n"+
                     format("oldModule = (%s) old.getModule();\n",moduleFQN)+
                 "} catch(Exception e) {\n"+
@@ -150,14 +151,20 @@ public class AbsFactoryGeneratedObjectFactory {
     private static String getServiceIfcsInitialization(List<FullyQualifiedName> providedServices) {
         String generic = format("Class<? extends %s>", AbstractServiceInterface.class.getCanonicalName());
 
-        String result = format("static {\n"+
-            "java.util.Set<%1$s> serviceIfcs2 = new java.util.HashSet<%1$s>();\n", generic);
+        String result = "static {\n";
+        if (!providedServices.isEmpty()) {
+            // TODO The generic specifier in HashSet<> isn't necessary, but the Eclipse AST parser used in the unit tests doesn't support this
+            result += format("java.util.Set<%1$s> serviceIfcs2 = new java.util.HashSet<%1$s>();\n", generic);
 
-        for(FullyQualifiedName fqn: providedServices) {
-            result += format("serviceIfcs2.add(%s.class);\n", fqn);
+            for(FullyQualifiedName fqn: providedServices) {
+                result += format("serviceIfcs2.add(%s.class);\n", fqn);
+            }
+
+            result += "serviceIfcs = java.util.Collections.unmodifiableSet(serviceIfcs2);\n";
+        } else {
+            result += "serviceIfcs = java.util.Collections.emptySet();\n";
         }
-        result += "serviceIfcs = java.util.Collections.unmodifiableSet(serviceIfcs2);\n"+
-            "}\n";
+        result += "}\n";
 
         // add isModuleImplementingServiceInterface and getImplementedServiceIntefaces methods
 
index 92a0eec..7118405 100644 (file)
@@ -149,8 +149,8 @@ public class AbsModuleGeneratedObjectFactory {
         return "\n"+
             "@Override\n"+
             "public boolean equals(Object o) {\n"+
-                "if (this == o) return true;\n"+
-                "if (o == null || getClass() != o.getClass()) return false;\n"+
+                "if (this == o) { return true; }\n"+
+                "if (o == null || getClass() != o.getClass()) { return false; }\n"+
                 format("%s that = (%1$s) o;\n", abstractFQN.getTypeName())+
                 "return identifier.equals(that.identifier);\n"+
             "}\n"+
@@ -182,7 +182,7 @@ public class AbsModuleGeneratedObjectFactory {
 
         for (ModuleField moduleField : moduleFields) {
             result += format(
-                "if (java.util.Objects.deepEquals(%s, other.%1$s) == false) {\n"+
+                "if (!java.util.Objects.deepEquals(%s, other.%1$s)) {\n"+
                     "return false;\n"+
                 "}\n", moduleField.getName());
 
index 4d08f53..e597bba 100644 (file)
@@ -67,7 +67,7 @@ public class ConcreteModuleGeneratedObjectFactory {
         // parameters
         stringBuilder.append(Joiner.on(", ").withKeyValueSeparator(" ").join(parameters));
         stringBuilder.append(") {\n");
-        if (parameters.isEmpty() == false) {
+        if (!parameters.isEmpty()) {
             stringBuilder.append("super(");
             stringBuilder.append(Joiner.on(", ").join(parameters.values()));
             stringBuilder.append(");\n");
index 40903a0..c2aeb89 100644 (file)
@@ -29,7 +29,7 @@ public class GeneratedObjectBuilder {
         content.append(maybeAddComment(input.getCopyright()));
         content.append(maybeAddComment(input.getHeader()));
 
-        if (input.getFQN().getPackageName().isEmpty() == false) {
+        if (!input.getFQN().getPackageName().isEmpty()) {
             content.append("package ");
             content.append(input.getFQN().getPackageName());
             content.append(";\n");
index 0b833f0..02ab91b 100644 (file)
@@ -85,14 +85,14 @@ public class StringUtil {
 
         int basicIndent = 4;
         StringBuilder sb = new StringBuilder();
-        int intends = 0, empty = 0;
+        int indents = 0, empty = 0;
         for (String line : split) {
-            intends -= StringUtils.countMatches(line, "}");
-            if (intends < 0) {
-                intends = 0;
+            indents -= StringUtils.countMatches(line, "}");
+            if (indents < 0) {
+                indents = 0;
             }
-            if (line.isEmpty() == false) {
-                sb.append(Strings.repeat(" ", basicIndent * intends));
+            if (!line.isEmpty()) {
+                sb.append(Strings.repeat(" ", basicIndent * indents));
                 sb.append(line);
                 sb.append("\n");
                 empty = 0;
@@ -102,7 +102,7 @@ public class StringUtil {
                     sb.append("\n");
                 }
             }
-            intends += StringUtils.countMatches(line, "{");
+            indents += StringUtils.countMatches(line, "{");
         }
         return ensureEndsWithSingleNewLine(sb.toString());
     }