Cleanup IDL Generator

Clean up pychecker warnings.
Move common code from idl_c_header (header generator) to idl_c_proto, the
'C' code style prototyping helper.

BUG= http://code.google.com/p/chromium/issues/detail?id=91607
TEST= cd src/ppapi/generators && ./generator.py
Review URL: http://codereview.chromium.org/8538029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@109958 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 104 insertions, 82 deletions

diff --git a/ppapi/generators/idl_ast.py b/ppapi/generators/idl_ast.py
index f215dbb..609253e 100644
--- a/ppapi/generators/idl_ast.py
+++ b/ppapi/generators/idl_ast.py
@@ -105,16 +105,7 @@ class IDLFileTypeResolver(IDLVisitor):
 #
 class IDLAst(IDLNode):
   def __init__(self, children):
-    objs = []
-
-    builtin = None
-    extranodes = []
-    for filenode in children:
-      if filenode.GetProperty('NAME') == 'pp_stdint.idl':
-        builtin = filenode
-        break
-
-    IDLNode.__init__(self, 'AST', 'BuiltIn', 1, 0, extranodes + children)
+    IDLNode.__init__(self, 'AST', 'BuiltIn', 1, 0, children)
     self.Resolve()
 
   def Resolve(self):
diff --git a/ppapi/generators/idl_c_header.py b/ppapi/generators/idl_c_header.py
index 7145a91..e08316c 100644
--- a/ppapi/generators/idl_c_header.py
+++ b/ppapi/generators/idl_c_header.py
@@ -9,7 +9,6 @@
 import glob
 import os
 import sys
-import subprocess
 
 from idl_log import ErrOut, InfoOut, WarnOut
 from idl_node import IDLAttribute, IDLNode
@@ -24,6 +23,7 @@ Option('dstroot', 'Base directory of output', default=os.path.join('..', 'c'))
 Option('guard', 'Include guard prefix', default=os.path.join('ppapi', 'c'))
 Option('out', 'List of output files', default='')
 
+
 def GetOutFileName(filenode, relpath=None, prefix=None):
   path, name = os.path.split(filenode.GetProperty('NAME'))
   name = os.path.splitext(name)[0] + '.h'
@@ -32,6 +32,7 @@ def GetOutFileName(filenode, relpath=None, prefix=None):
   if relpath: name = os.path.join(relpath, name)
   return name
 
+
 def WriteGroupMarker(out, node, last_group):
   # If we are part of a group comment marker...
   if last_group and last_group != node.cls:
@@ -50,7 +51,6 @@ def WriteGroupMarker(out, node, last_group):
 
 
 def GenerateHeader(out, filenode, releases):
-  gpath = GetOption('guard')
   cgen = CGen()
   pref = ''
   do_comments = True
@@ -103,34 +103,16 @@ class HGen(GeneratorByFile):
   def __init__(self):
     Generator.__init__(self, 'C Header', 'cgen', 'Generate the C headers.')
 
-  def GetMacro(self, node):
-    name = node.GetName()
-    name = name.upper()
-    return "%s_INTERFACE" % name
-
-  def GetDefine(self, name, value):
-    out = '#define %s %s' % (name, value)
-    if len(out) > 80:
-      out = '#define %s \\\n    %s' % (name, value)
-    return '%s\n' % out
-
-  def GetVersionString(self, node):
-    # If an interface name is specified, use that
-    iname = node.GetProperty('iname')
-    if iname: return iname
-
-    # Otherwise, the interface name is the object's name
-    # With '_Dev' replaced by '(Dev)' if it's a Dev interface.
-    name = node.GetName()
-    if len(name) > 4 and name[-4:] == '_Dev':
-      name = '%s(Dev)' % name[:-4]
-    return name
-
-  def GetOutFile(self, filenode, options):
+  def GenerateFile(self, filenode, releases, options):
     savename = GetOutFileName(filenode, GetOption('dstroot'))
-    return IDLOutFile(savename)
+    out = IDLOutFile(savename)
+    self.GenerateHead(out, filenode, releases, options)
+    self.GenerateBody(out, filenode, releases, options)
+    self.GenerateTail(out, filenode, releases, options)
+    return out.Close()
 
   def GenerateHead(self, out, filenode, releases, options):
+    __pychecker__ = 'unusednames=options'
     cgen = CGen()
     gpath = GetOption('guard')
     release = releases[0]
@@ -176,27 +158,24 @@ class HGen(GeneratorByFile):
     out.Write('\n')
     for node in filenode.GetListOf('Interface'):
       idefs = ''
-      name = self.GetVersionString(node)
-      macro = node.GetProperty('macro')
-      if not macro:
-        macro = self.GetMacro(node)
-
-      unique = node.GetUniqueReleases(releases)
-      for rel in unique:
+      macro = cgen.GetInterfaceMacro(node)
+      for rel in node.GetUniqueReleases(releases):
         version = node.GetVersion(rel)
+        name = cgen.GetInterfaceString(node, version)
         strver = str(version).replace('.', '_')
-        idefs += self.GetDefine('%s_%s' % (macro, strver),
-                                '"%s;%s"' % (name, version))
-      idefs += self.GetDefine(macro, '%s_%s' % (macro, strver)) + '\n'
+        idefs += cgen.GetDefine('%s_%s' % (macro, strver), '"%s"' % name)
+      idefs += cgen.GetDefine(macro, '%s_%s' % (macro, strver)) + '\n'
       out.Write(idefs)
 
     # Generate the @file comment
     out.Write('%s\n' % Comment(fileinfo, prefix='*\n @file'))
 
   def GenerateBody(self, out, filenode, releases, options):
+    __pychecker__ = 'unusednames=options'
     GenerateHeader(out, filenode, releases)
 
   def GenerateTail(self, out, filenode, releases, options):
+    __pychecker__ = 'unusednames=options,releases'
     gpath = GetOption('guard')
     def_guard = GetOutFileName(filenode, relpath=gpath)
     def_guard = def_guard.replace(os.sep,'_').replace('.','_').upper() + '_'
diff --git a/ppapi/generators/idl_c_proto.py b/ppapi/generators/idl_c_proto.py
index 0a4e0df..f3eab9d 100644
--- a/ppapi/generators/idl_c_proto.py
+++ b/ppapi/generators/idl_c_proto.py
@@ -58,7 +58,7 @@ def Comment(node, prefix=None, tabs=0):
       lines = prefix_lines + lines;
   return CommentLines(lines, tabs)
 
-def GetNodeComments(node, prefix=None, tabs=0):
+def GetNodeComments(node, tabs=0):
   # Generate a comment block joining all comment nodes which are children of
   # the provided node.
   comment_txt = ''
@@ -158,8 +158,7 @@ class CGen(object):
   #
   def Log(self, txt):
     if not GetOption('cgen_debug'): return
-    tabs = ''
-    for tab in range(self.dbg_depth): tabs += '  '
+    tabs = '  ' * self.dbg_depth
     print '%s%s' % (tabs, txt)
 
   def LogEnter(self, txt):
@@ -170,12 +169,48 @@ class CGen(object):
     self.dbg_depth -= 1
     if txt: self.Log(txt)
 
+
+  def GetDefine(self, name, value):
+    out = '#define %s %s' % (name, value)
+    if len(out) > 80:
+      out = '#define %s \\\n    %s' % (name, value)
+    return '%s\n' % out
+
+  #
+  # Interface strings
+  #
+  def GetMacroHelper(self, node):
+    macro = node.GetProperty('macro')
+    if macro: return macro
+    name = node.GetName()
+    name = name.upper()
+    return "%s_INTERFACE" % name
+
+  def GetInterfaceMacro(self, node, version = None):
+    name = self.GetMacroHelper(node)
+    if version is None:
+      return name
+    return '%s_%s' % (name, str(version).replace('.', '_'))
+
+  def GetInterfaceString(self, node, version = None):
+    # If an interface name is specified, use that
+    name = node.GetProperty('iname')
+    if not name:
+      # Otherwise, the interface name is the object's name
+      # With '_Dev' replaced by '(Dev)' if it's a Dev interface.
+      name = node.GetName()
+      if name.endswith('_Dev'):
+        name = '%s(Dev)' % name[:-4]
+    if version is None:
+      return name
+    return "%s;%s" % (name, version)
+
+
   #
   # Return the array specification of the object.
   #
   def GetArraySpec(self, node):
     assert(node.cls == 'Array')
-    out = ''
     fixed = node.GetProperty('FIXED')
     if fixed:
       return '[%s]' % fixed
@@ -367,6 +402,7 @@ class CGen(object):
 
   # Define a Typedef.
   def DefineTypedef(self, node, releases, prefix='', comment=False):
+    __pychecker__ = 'unusednames=comment'
     release = releases[0]
     out = 'typedef %s;\n' % self.GetSignature(node, release, 'return',
                                               prefix, True)
@@ -375,6 +411,7 @@ class CGen(object):
 
   # Define an Enum.
   def DefineEnum(self, node, releases, prefix='', comment=False):
+    __pychecker__ = 'unusednames=comment,releases'
     self.LogEnter('DefineEnum %s' % node)
     unnamed =  node.GetProperty('unnamed')
     if unnamed:
@@ -400,6 +437,7 @@ class CGen(object):
     return out
 
   def DefineMember(self, node, releases, prefix='', comment=False):
+    __pychecker__ = 'unusednames=prefix,comment'
     release = releases[0]
     self.LogEnter('DefineMember %s' % node)
     out = '%s;' % self.GetSignature(node, release, 'store', '', True)
@@ -425,6 +463,7 @@ class CGen(object):
 
 
   def DefineStruct(self, node, releases, prefix='', comment=False):
+    __pychecker__ = 'unusednames=comment,prefix'
     self.LogEnter('DefineStruct %s' % node)
     out = ''
     build_list = node.GetUniqueReleases(releases)
@@ -459,17 +498,17 @@ class CGen(object):
       return ''
 
     self.LogEnter('Define %s tab=%d prefix="%s"' % (node,tabs,prefix))
-    declmap = {
-      'Enum' : CGen.DefineEnum,
-      'Function' : CGen.DefineMember,
-      'Interface' : CGen.DefineStruct,
-      'Member' : CGen.DefineMember,
-      'Struct' : CGen.DefineStruct,
-      'Typedef' : CGen.DefineTypedef,
-    }
+    declmap = dict({
+      'Enum': CGen.DefineEnum,
+      'Function': CGen.DefineMember,
+      'Interface': CGen.DefineStruct,
+      'Member': CGen.DefineMember,
+      'Struct': CGen.DefineStruct,
+      'Typedef': CGen.DefineTypedef
+    })
 
     out = ''
-    func = declmap.get(node.cls)
+    func = declmap.get(node.cls, None)
     if not func:
       ErrOut.Log('Failed to define %s named %s' % (node.cls, node.GetName()))
     define_txt = func(self, node, releases, prefix=prefix, comment=comment)
diff --git a/ppapi/generators/idl_generator.py b/ppapi/generators/idl_generator.py
index e6149ec..b6372ee 100644
--- a/ppapi/generators/idl_generator.py
+++ b/ppapi/generators/idl_generator.py
@@ -8,6 +8,7 @@ import sys
 
 from idl_log import ErrOut, InfoOut, WarnOut
 from idl_option import GetOption, Option, ParseOptions
+from idl_parser import ParseFiles
 
 GeneratorList = []
 
@@ -39,6 +40,7 @@ class Generator(object):
                              default='')
     GeneratorList.append(self)
     self.errors = 0
+    self.skip_list = []
 
   def Error(self, msg):
     ErrOut.Log('Error %s : %s' % (self.name, msg))
@@ -68,6 +70,13 @@ class Generator(object):
 
     print "Found releases: %s" % ast.releases
 
+    # Generate list of files to ignore due to errors
+    for filenode in ast.GetListOf('File'):
+      # If this file has errors, skip it
+      if filenode.GetProperty('ERRORS') > 0:
+        self.skip_list.append(filenode)
+        continue
+
     # Check for a range option which over-rides a release option
     if not releasestr and rangestr:
       range_list = rangestr.split(',')
@@ -86,9 +95,9 @@ class Generator(object):
 
         vmin = ast.releases.index(vmin)
         vmax = ast.releases.index(vmax) + 1
-        range = ast.releases[vmin:vmax]
+        releases = ast.releases[vmin:vmax]
         InfoOut.Log('Generate range %s of %s.' % (range, self.name))
-        ret = self.GenerateRange(ast, range, options)
+        ret = self.GenerateRange(ast, releases, options)
         if ret < 0:
           self.Error('Failed to generate range %s : %s.' %(vmin, vmax))
         else:
@@ -112,8 +121,8 @@ class Generator(object):
     self.Error("Undefined release generator.")
     return 0
 
-  def GenerateRange(self, ast, vmin, vmax, options):
-    __pychecker__ = 'unusednames=ast,vmin,vmax,options'
+  def GenerateRange(self, ast, releases, options):
+    __pychecker__ = 'unusednames=ast,releases,options'
     self.Error("Undefined range generator.")
     return 0
 
@@ -143,6 +152,11 @@ class Generator(object):
 #  GenerateTail - Writes the end of the file (closing include guard, etc...)
 #
 class GeneratorByFile(Generator):
+  def GenerateFile(self, filenode, releases, options):
+    __pychecker__ = 'unusednames=filenode,releases,options'
+    self.Error("Undefined release generator.")
+    return 0
+
   def GenerateRelease(self, ast, release, options):
     return self.GenerateRange(ast, [release], options)
 
@@ -154,22 +168,17 @@ class GeneratorByFile(Generator):
     skipList = []
     cnt = 0
     for filenode in ast.GetListOf('File'):
-      # Skip this file if not required
-      if outlist and filenode.GetName() not in outlist:
+      # Ignore files with errors
+      if filenode in self.skip_list:
         continue
 
-      # If this file has errors, skip it
-      if filenode.GetProperty('ERRORS') > 0:
-        skipList.append(filenode)
+      # Skip this file if not required
+      if outlist and filenode.GetName() not in outlist:
         continue
 
-      # Create output file
-      out = self.GetOutFile(filenode, options)
-      self.GenerateHead(out, filenode, releases, options)
-      self.GenerateBody(out, filenode, releases, options)
-      self.GenerateTail(out, filenode, releases, options)
-
-      if out.Close(): cnt = cnt + 1
+      # Create the output file and increment out count if there was a delta
+      if self.GenerateFile(filenode, releases, options):
+        cnt = cnt + 1
 
     for filenode in skipList:
       errcnt = filenode.GetProperty('ERRORS')
@@ -208,8 +217,8 @@ class GeneratorReleaseTest(Generator):
       check_release = 0
     return check_release == 1
 
-  def GenerateRange(self, ast, vmin, vmax, options = {}):
-    __pychecker__ = 'unusednames=ast,vmin,vmax,options'
+  def GenerateRange(self, ast, releases, options):
+    __pychecker__ = 'unusednames=ast,releases,options'
     global check_range
     check_range = 1
     return True
@@ -244,7 +253,6 @@ def Test():
 
 def Main(args):
   if not args: return Test()
-
   filenames = ParseOptions(args)
   ast = ParseFiles(filenames)
 
diff --git a/ppapi/generators/idl_node.py b/ppapi/generators/idl_node.py
index 3ae4021..1372d38 100644
--- a/ppapi/generators/idl_node.py
+++ b/ppapi/generators/idl_node.py
@@ -244,6 +244,12 @@ class IDLNode(IDLRelease):
       return None
     return filenode.release_map.GetVersion(release)
 
+  def GetRelease(self, version):
+    filenode = self.GetProperty('FILE')
+    if not filenode:
+      return None
+    return filenode.release_map.GetRelease(version)
+
   def GetUniqueReleases(self, releases):
     # Given a list of global release, return a subset of releases
     # for this object that change.
diff --git a/ppapi/generators/idl_parser.py b/ppapi/generators/idl_parser.py
index 60b0050..4b4417a 100644
--- a/ppapi/generators/idl_parser.py
+++ b/ppapi/generators/idl_parser.py
@@ -1005,13 +1005,12 @@ default_dirs = ['.', 'trusted', 'dev']
 def ParseFiles(filenames):
   parser = IDLParser()
   filenodes = []
-  errors = 0
 
   if not filenames:
     filenames = []
     srcroot = GetOption('srcroot')
-    for dir in default_dirs:
-      srcdir = os.path.join(srcroot, dir, '*.idl')
+    for dirname in default_dirs:
+      srcdir = os.path.join(srcroot, dirname, '*.idl')
       srcdir = os.path.normpath(srcdir)
       filenames += sorted(glob.glob(srcdir))