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 4ad8fd79c46fa8ad76088a2ef6ec3ea2289d5e5d..2216f19f884584ab0a4fd5762034817735ce4638 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 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;
     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");
 
                 .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();
 
         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
             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(
                     throw new IllegalStateException(
-                        "Cannot add two SIE  with same qname "
+                        "Cannot add two SIE with same qname "
                                 + sieEntry.getValue());
                 }
             }
                                 + 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;
                 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);
             }
         }
                 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) {
 
     @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
     }
 
     @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);
     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(
     }
 
     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 (files.contains(file)) {
                 List<File> undeletedFiles = Lists.newArrayList();
                 for (File presentFile : files) {
-                    if (presentFile.delete() == false) {
+                    if (!presentFile.delete()) {
                         undeletedFiles.add(presentFile);
                     }
                 }
                         undeletedFiles.add(presentFile);
                     }
                 }
-                if (undeletedFiles.isEmpty() == false) {
+                if (!undeletedFiles.isEmpty()) {
                     LOG.error(
                             "Illegal state occurred: Unable to delete already generated files, undeleted files: {}",
                             undeletedFiles);
                     LOG.error(
                             "Illegal state occurred: Unable to delete already generated files, undeleted files: {}",
                             undeletedFiles);
index 2e867d307fa8b25d14c0d6a7611d19a3dd21ccc1..68d0b717a994d73240cc79f5f57f042cca7572a5 100644 (file)
@@ -25,13 +25,13 @@ public interface FtlTemplate {
 
     Optional<String> getMaybeJavadoc();
 
 
     Optional<String> getMaybeJavadoc();
 
-    public List<Annotation> getAnnotations();
+    List<Annotation> getAnnotations();
 
     TypeDeclaration getTypeDeclaration();
 
 
     TypeDeclaration getTypeDeclaration();
 
-    public String getFullyQualifiedName();
+    String getFullyQualifiedName();
 
 
-    public List<Field> getFields();
+    List<Field> getFields();
 
     List<? extends Method> getMethods();
 
 
     List<? extends Method> getMethods();
 
index 13345d62a4dd1bc96a69382f79b812c1e1f6a8d7..27fb2bcba153f953ebd0559fc8a9ee377b5e7ca8 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 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;
 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 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(
     }
 
     public static RuntimeBeanEntry findRoot(
@@ -65,7 +64,7 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
 
     private static String constructConstructorBody(
             List<Field> constructorParameters) {
 
     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());
         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
             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(),
                             .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",
 
             MethodDefinition registerMethod = new MethodDefinition(
                     childRegistratorFQN, "register",
-                    Arrays.asList(rbParameter), registerBody.toString());
+                    Collections.singletonList(rbParameter), registerBody.toString());
             methods.add(registerMethod);
         }
 
             methods.add(registerMethod);
         }
 
@@ -128,11 +127,11 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
         // TODO add header
         GeneralClassTemplate registrator = new GeneralClassTemplate(null,
                 rootRB.getPackageName(), registratorName,
         // 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);
                 + registrator.getTypeDeclaration().getName());
         Map<String, FtlTemplate> result = new HashMap<>();
         result.putAll(RuntimeRegistratorFtlTemplates);
@@ -140,7 +139,7 @@ public class RuntimeRegistratorFtlTemplate extends GeneralClassTemplate {
         return result;
     }
 
         return result;
     }
 
-    private static Field hierachchicalRegistration = new Field(
+    private static Field hierachicalRegistration = new Field(
             Lists.newArrayList("final"),
             HierarchicalRuntimeBeanRegistration.class.getCanonicalName(),
             "registration");
             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());
 
         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,
                 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
                 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()));
                                 .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(
                 Field param = new Field(Lists.newArrayList("final"),
                         child.getJavaNameOfRuntimeMXBean(), "bean");
                 MethodDefinition register = new MethodDefinition(
-                        Arrays.asList("synchronized"),
+                        Collections.singletonList("synchronized"),
                         childRegistrator.getFullyQualifiedName(), "register",
                         childRegistrator.getFullyQualifiedName(), "register",
-                        Arrays.asList(param), Collections.<String> emptyList(),
+                        Collections.singletonList(param), Collections.<String> emptyList(),
                         Collections.<Annotation> emptyList(), body.toString());
                 methods.add(register);
 
                         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());
 
         // 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);
 
         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);
         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);
 
         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;
                 + created.getTypeDeclaration().getName());
         result.putAll(unorderedResult);
         return result;
index 1eaa35f7813f5d3482c92b745dc67f99e66346ac..0030c3c03806eb5837335ff624cb2c8a5858a68c 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 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;
 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();
         { // 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<>();
 
                     .getCanonicalName());
             List<MethodDeclaration> methods = new ArrayList<>();
 
@@ -179,8 +178,7 @@ public class TemplateFactory {
     public static AbstractFactoryTemplate abstractFactoryTemplateFromMbe(
             final ModuleMXBeanEntry mbe) {
         AbstractFactoryAttributesProcessor attrProcessor = new AbstractFactoryAttributesProcessor();
     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
         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());
             generateRuntime = true;
             RuntimeBeanEntry rootEntry = RuntimeRegistratorFtlTemplate
                     .findRoot(mbe.getRuntimeBeans());
@@ -294,7 +292,7 @@ public class TemplateFactory {
                 continue;
             }
 
                 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);
         }
                     "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(
 
             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()) {
                 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();
 
 
         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;
                 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());
                 }
                 } else {
                     throw new UnsupportedOperationException(
                             "Attribute not supported: "
                                     + attributeIfc.getClass());
                 }
-
-                fields.add(new Field(type, attributeIfc
-                        .getUpperCaseCammelCase(), nullableDefaultWrapped));
             }
         }
 
             }
         }
 
index e097516943fb6a5bbc3736223dde6a00c07b6d1e..468d20c0db2ae98f4b734b5bfa26c0d775565945 100644 (file)
@@ -14,7 +14,7 @@ class TypeHelper {
      * "List<String>" for input parameters List.class, String.class
      */
     static String getGenericType(Class<?> type, Class<?>... parameters) {
      * "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("<");
         sb.append(type.getCanonicalName());
         if (parameters.length > 0) {
             sb.append("<");
index c257aa6dda61e382a04b0e2c62ac5be525cac00d..23d3ac254fdd686b6b80c793523015813a69eb0c 100644 (file)
@@ -16,7 +16,7 @@ public class ConstructorSerializer {
         if (constr.isPublic()) {
             build.append("public ");
         }
         if (constr.isPublic()) {
             build.append("public ");
         }
-        build.append(constr.getTypeName() + " ");
+        build.append(constr.getTypeName());
         build.append("() {");
         build.append("\n");
         build.append("    ");
         build.append("() {");
         build.append("\n");
         build.append("    ");
index 90a2b5694c0f2df3897b5361278cae62454f13fc..7105c39fe7d5863594fc3cacdef15e883b8ea50d 100644 (file)
@@ -14,12 +14,12 @@ public class FieldSerializer {
         StringBuilder build = new StringBuilder();
         build.append("private ");
         for (String mod : field.getModifiers()) {
         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.getName());
         if (field.getDefinition() != null) {
-            build.append(" = " + field.getDefinition());
+            build.append(" = ").append(field.getDefinition());
         }
         build.append(";");
         build.append("\n");
         }
         build.append(";");
         build.append("\n");
index 89a2cf7b90418e0d634fc2a8ebac3b041689dd7e..5eabeabd0da32faa54367f4813d47fa6b82484e9 100644 (file)
@@ -9,35 +9,22 @@
 package org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model;
 
 import java.util.Date;
 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) {
 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 7d2b9fcdafa1974be14d512deba8df25c17a3140..eba6e53e23257899ccc178760cd3bc30b8e676f6 100644 (file)
@@ -24,42 +24,40 @@ class MethodSerializer {
 
         build.append("    " + "public ");
         for (String mod : method.getModifiers()) {
 
         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()) {
         for (Field param : method.getParameters()) {
+            if (!firstParam) {
+                build.append(", ");
+            }
             for (String mod : param.getModifiers()) {
             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 (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 ");
             }
                 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(" {");
             build.append("\n");
             build.append("        ");
-            build.append(((MethodDefinition) method).getBody());
+            build.append(definition.getBody());
             build.append("\n");
             build.append("    ");
             build.append("}");
             build.append("\n");
             build.append("    ");
             build.append("}");
index 6211efe417973ee0f8faf637669e4f824a1df7f3..abb3e4ccafb4d8ad684c4eef542aca6485d7e03f 100644 (file)
@@ -16,28 +16,32 @@ public class ModuleFieldSerializer {
     public static String toString(ModuleField moduleField) {
         StringBuilder builder = new StringBuilder();
         builder.append("    ");
     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("\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.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(";");
 
         if (moduleField.isDependent()) {
             String comment = moduleField.getDependency().isMandatory() ? "mandatory"
                     : "optional";
-            builder.append(" // " + comment);
+            builder.append(" // ").append(comment);
         }
         builder.append("\n");
 
         }
         builder.append("\n");
 
index 8c65eaad14247a8be963c1dbc1d3095c7427d4d9..25314cb5f5fbe963488633bb50c6285ae58bd5a2 100644 (file)
@@ -114,11 +114,12 @@ public class AbsFactoryGeneratedObjectFactory {
                 "throw new UnsupportedOperationException(\"Class reloading is not supported\");\n"+
             "}\n", moduleFQN, DynamicMBeanWithInstance.class.getCanonicalName()));
 
                 "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"+
         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();
     }
 
         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("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"+
                 "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());
 
     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
 
 
         // add isModuleImplementingServiceInterface and getImplementedServiceIntefaces methods
 
index 92a0eec5866877834f4fef563e9d3a3e06acc3aa..7118405deaff7559ec7c60777a8da1c8f0055aa3 100644 (file)
@@ -149,8 +149,8 @@ public class AbsModuleGeneratedObjectFactory {
         return "\n"+
             "@Override\n"+
             "public boolean equals(Object o) {\n"+
         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"+
                 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(
 
         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());
 
                     "return false;\n"+
                 "}\n", moduleField.getName());
 
index 4d08f53ee4bbaae41204f4e6dff99276f5a12432..e597bba8bb1f6fb959d6880195d7fc4ea898da4d 100644 (file)
@@ -67,7 +67,7 @@ public class ConcreteModuleGeneratedObjectFactory {
         // parameters
         stringBuilder.append(Joiner.on(", ").withKeyValueSeparator(" ").join(parameters));
         stringBuilder.append(") {\n");
         // 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");
             stringBuilder.append("super(");
             stringBuilder.append(Joiner.on(", ").join(parameters.values()));
             stringBuilder.append(");\n");
index 40903a05fc73201278b881d5d22fd994c67559aa..c2aeb89c2d494d117b2310be7248259ff01b99b5 100644 (file)
@@ -29,7 +29,7 @@ public class GeneratedObjectBuilder {
         content.append(maybeAddComment(input.getCopyright()));
         content.append(maybeAddComment(input.getHeader()));
 
         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");
             content.append("package ");
             content.append(input.getFQN().getPackageName());
             content.append(";\n");
index 0b833f01b9cb401468cefed184a324c96a1516cd..02ab91b817cb20bda841c4cc671076d41cba49da 100644 (file)
@@ -85,14 +85,14 @@ public class StringUtil {
 
         int basicIndent = 4;
         StringBuilder sb = new StringBuilder();
 
         int basicIndent = 4;
         StringBuilder sb = new StringBuilder();
-        int intends = 0, empty = 0;
+        int indents = 0, empty = 0;
         for (String line : split) {
         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;
                 sb.append(line);
                 sb.append("\n");
                 empty = 0;
@@ -102,7 +102,7 @@ public class StringUtil {
                     sb.append("\n");
                 }
             }
                     sb.append("\n");
                 }
             }
-            intends += StringUtils.countMatches(line, "{");
+            indents += StringUtils.countMatches(line, "{");
         }
         return ensureEndsWithSingleNewLine(sb.toString());
     }
         }
         return ensureEndsWithSingleNewLine(sb.toString());
     }