Refactor type checking to allow mandatory attribute to be dropped

4 files changed, 251 insertions, 166 deletions

diff --git a/src/luarocks/type/manifest.lua b/src/luarocks/type/manifest.lua
index f58ff984..407428b2 100644
--- a/src/luarocks/type/manifest.lua
+++ b/src/luarocks/type/manifest.lua
@@ -2,59 +2,57 @@ local type_manifest = {}
 
 local type_check = require("luarocks.type_check")
 
-local number_1 = type_check.number_1
-local string_1 = type_check.string_1
-local mandatory_string_1 = type_check.mandatory_string_1
-
-local manifest_types = {
-   repository = {
-      _mandatory = true,
-      -- packages
-      _any = {
-         -- versions
+local manifest_formats = type_check.declare_schemas({
+   ["1.0"] = {
+      repository = {
+         _mandatory = true,
+         -- packages
          _any = {
-            -- items
+            -- versions
             _any = {
-               arch = mandatory_string_1,
-               modules = { _any = string_1 },
-               commands = { _any = string_1 },
-               dependencies = { _any = string_1 },
-               -- TODO: to be extended with more metadata.
+               -- items
+               _any = {
+                  arch = { _type = "string", _mandatory = true },
+                  modules = { _any = { _type = "string" } },
+                  commands = { _any = { _type = "string" } },
+                  dependencies = { _any = { _type = "string" } },
+                  -- TODO: to be extended with more metadata.
+               }
             }
          }
-      }
-   },
-   modules = {
-      _mandatory = true,
-      -- modules
-      _any = {
-         -- providers
-         _any = string_1
-      }
-   },
-   commands = {
-      _mandatory = true,
-      -- modules
-      _any = {
-         -- commands
-         _any = string_1
-      }
-   },
-   dependencies = {
-      -- each module
-      _any = {
-         -- each version
+      },
+      modules = {
+         _mandatory = true,
+         -- modules
          _any = {
-            -- each dependency
+            -- providers
+            _any = { _type = "string" }
+         }
+      },
+      commands = {
+         _mandatory = true,
+         -- modules
+         _any = {
+            -- commands
+            _any = { _type = "string" }
+         }
+      },
+      dependencies = {
+         -- each module
+         _any = {
+            -- each version
             _any = {
-               name = string_1,
-               constraints = {
-                  _any = {
-                     no_upgrade = { _type = "boolean" },
-                     op = string_1,
-                     version = {
-                        string = string_1,
-                        _any = number_1,
+               -- each dependency
+               _any = {
+                  name = { _type = "string" },
+                  constraints = {
+                     _any = {
+                        no_upgrade = { _type = "boolean" },
+                        op = { _type = "string" },
+                        version = {
+                           string = { _type = "string" },
+                           _any = { _type = "number" },
+                        }
                      }
                   }
                }
@@ -62,8 +60,7 @@ local manifest_types = {
          }
       }
    }
-}
-
+})
 
 --- Type check a manifest table.
 -- Verify the correctness of elements from a 
@@ -73,9 +70,10 @@ local manifest_types = {
 -- succeeded, or nil and an error message if it failed.
 function type_manifest.check(manifest, globals)
    assert(type(manifest) == "table")
-   local ok, err = type_check.check_undeclared_globals(globals, manifest_types)
+   local format = manifest_formats["1.0"]
+   local ok, err = type_check.check_undeclared_globals(globals, format)
    if not ok then return nil, err end
-   return type_check.type_check_table("1.0", manifest, manifest_types, "")
+   return type_check.type_check_table("1.0", manifest, format, "")
 end
 
 return type_manifest
diff --git a/src/luarocks/type/rockspec.lua b/src/luarocks/type/rockspec.lua
index 16ab911c..8b2bdffa 100644
--- a/src/luarocks/type/rockspec.lua
+++ b/src/luarocks/type/rockspec.lua
@@ -4,18 +4,11 @@ local type_check = require("luarocks.type_check")
 
 type_rockspec.rockspec_format = "3.0"
 
-local string_1 = type_check.string_1
-local mandatory_string_1 = type_check.mandatory_string_1
-
-local string_3 = { _type = "string", _version = "3.0" }
-local list_of_strings_3 = { _any = string_3, _version = "3.0" }
-
 -- Syntax for type-checking tables:
 --
 -- A type-checking table describes typing data for a value.
 -- Any key starting with an underscore has a special meaning:
 -- _type (string) is the Lua type of the value. Default is "table".
--- _version (string) is the minimum rockspec_version that supports this value. Default is "1.0".
 -- _mandatory (boolean) indicates if the value is a mandatory key in its container table. Default is false.
 -- For "string" types only:
 --    _pattern (string) is the string-matching pattern, valid for string types only. Default is ".*".
@@ -24,110 +17,123 @@ local list_of_strings_3 = { _any = string_3, _version = "3.0" }
 --    _more (boolean) indicates that the table accepts unspecified keys and does not type-check them.
 --    Any other string keys that don't start with an underscore represent known keys and are type-checking tables, recursively checked.
 
-local rockspec_types = {
-   rockspec_format = string_1,
-   package = mandatory_string_1,
-   version = { _type = "string", _pattern = "[%w.]+-[%d]+", _mandatory = true },
-   description = {
-      summary = string_1,
-      detailed = string_1,
-      homepage = string_1,
-      license = string_1,
-      maintainer = string_1,
-      labels = list_of_strings_3,
-      issues_url = string_3,
-   },
-   dependencies = {
-      platforms = {}, -- recursively defined below
-      _any = {
-         _type = "string",
-         _name = "a valid dependency string",
-         _patterns = {
-            ["1.0"] = "%s*([a-zA-Z0-9][a-zA-Z0-9%.%-%_]*)%s*([^/]*)",
-            ["3.0"] = "%s*([a-zA-Z0-9%.%-%_]*/?[a-zA-Z0-9][a-zA-Z0-9%.%-%_]*)%s*([^/]*)",
+local rockspec_formats = type_check.declare_schemas({
+   ["1.0"] = {
+      rockspec_format = { _type = "string" },
+      package = { _type = "string", _mandatory = true },
+      version = { _type = "string", _pattern = "[%w.]+-[%d]+", _mandatory = true },
+      description = {
+         summary = { _type = "string" },
+         detailed = { _type = "string" },
+         homepage = { _type = "string" },
+         license = { _type = "string" },
+         maintainer = { _type = "string" },
+      },
+      dependencies = {
+         platforms = type_check.MAGIC_PLATFORMS,
+         _any = {
+            _type = "string",
+            _name = "a valid dependency string",
+            _pattern = "%s*([a-zA-Z0-9][a-zA-Z0-9%.%-%_]*)%s*([^/]*)",
          },
       },
-   },
-   build_dependencies = {
-      _version = "3.0",
-      platforms = {}, -- recursively defined below
-      _any = {
-         _type = "string",
-         _name = "a valid dependency string",
-         _pattern = "%s*([a-zA-Z0-9%.%-%_]*/?[a-zA-Z0-9][a-zA-Z0-9%.%-%_]*)%s*([^/]*)",
+      supported_platforms = {
+         _any = { _type = "string" },
       },
-   },
-   test_dependencies = {
-      _version = "3.0",
-      platforms = {}, -- recursively defined below
-      _any = {
-         _type = "string",
-         _name = "a valid dependency string",
-         _pattern = "%s*([a-zA-Z0-9%.%-%_]*/?[a-zA-Z0-9][a-zA-Z0-9%.%-%_]*)%s*([^/]*)",
+      external_dependencies = {
+         platforms = type_check.MAGIC_PLATFORMS,
+         _any = {
+            program = { _type = "string" },
+            header = { _type = "string" },
+            library = { _type = "string" },
+         }
+      },
+      source = {
+         _mandatory = true,
+         platforms = type_check.MAGIC_PLATFORMS,
+         url = { _type = "string", _mandatory = true },
+         md5 = { _type = "string" },
+         file = { _type = "string" },
+         dir = { _type = "string" },
+         tag = { _type = "string" },
+         branch = { _type = "string" },
+         module = { _type = "string" },
+         cvs_tag = { _type = "string" },
+         cvs_module = { _type = "string" },
+      },
+      build = {
+         platforms = type_check.MAGIC_PLATFORMS,
+         type = { _type = "string" },
+         install = {
+            lua = {
+               _more = true
+            },
+            lib = {
+               _more = true
+            },
+            conf = {
+               _more = true
+            },
+            bin = {
+               _more = true
+            }
+         },
+         copy_directories = {
+            _any = { _type = "string" },
+         },
+         _more = true,
+         _mandatory = true
+      },
+      hooks = {
+         platforms = type_check.MAGIC_PLATFORMS,
+         post_install = { _type = "string" },
       },
    },
-   supported_platforms = {
-      _any = string_1,
-   },
-   external_dependencies = {
-      platforms = {}, -- recursively defined below
-      _any = {
-         program = string_1,
-         header = string_1,
-         library = string_1,
+   
+   ["1.1"] = {
+      deploy = {
+         wrap_bin_scripts = { _type = "boolean" },
       }
    },
-   source = {
-      _mandatory = true,
-      platforms = {}, -- recursively defined below
-      url = mandatory_string_1,
-      md5 = string_1,
-      file = string_1,
-      dir = string_1,
-      tag = string_1,
-      branch = string_1,
-      module = string_1,
-      cvs_tag = string_1,
-      cvs_module = string_1,
-   },
-   build = {
-      platforms = {}, -- recursively defined below
-      type = string_1,
-      install = {
-         lua = {
-            _more = true
+   
+   ["3.0"] = {
+      description = {
+         labels = {
+            _any = { _type = "string" }
          },
-         lib = {
-            _more = true
+         issues_url = { _type = "string" },
+      },
+      dependencies = {
+         _any = {
+            _pattern = "%s*([a-zA-Z0-9%.%-%_]*/?[a-zA-Z0-9][a-zA-Z0-9%.%-%_]*)%s*([^/]*)",
          },
-         conf = {
-            _more = true
+      },
+      build_dependencies = {
+         platforms = type_check.MAGIC_PLATFORMS,
+         _any = {
+            _type = "string",
+            _name = "a valid dependency string",
+            _pattern = "%s*([a-zA-Z0-9%.%-%_]*/?[a-zA-Z0-9][a-zA-Z0-9%.%-%_]*)%s*([^/]*)",
+         },
+      },
+      test_dependencies = {
+         platforms = type_check.MAGIC_PLATFORMS,
+         _any = {
+            _type = "string",
+            _name = "a valid dependency string",
+            _pattern = "%s*([a-zA-Z0-9%.%-%_]*/?[a-zA-Z0-9][a-zA-Z0-9%.%-%_]*)%s*([^/]*)",
          },
-         bin = {
-            _more = true
-         }
       },
-      copy_directories = {
-         _any = string_1,
+      build = {
+         _mandatory = false,
+      },
+      test = {
+         platforms = type_check.MAGIC_PLATFORMS,
+         type = { _type = "string" },
+         _more = true,
       },
-      _more = true,
-      _mandatory = true
-   },
-   test = {
-      _version = "3.0",
-      platforms = {}, -- recursively defined below
-      type = string_1,
-      _more = true,
-   },
-   hooks = {
-      platforms = {}, -- recursively defined below
-      post_install = string_1,
-   },
-   deploy = {
-      _version = "1.1",
-      wrap_bin_scripts = { _type = "boolean", _version = "1.1" },
    }
-}
+})
 
 type_rockspec.order = {"rockspec_format", "package", "version", 
    { "source", { "url", "tag", "branch", "md5" } },
@@ -137,14 +143,6 @@ type_rockspec.order = {"rockspec_format", "package", "version",
    "test_dependencies", { "test", {"type"} },
    "hooks"}
 
-rockspec_types.build.platforms._any = rockspec_types.build
-rockspec_types.dependencies.platforms._any = rockspec_types.dependencies
-rockspec_types.build_dependencies.platforms._any = rockspec_types.build_dependencies
-rockspec_types.external_dependencies.platforms._any = rockspec_types.external_dependencies
-rockspec_types.source.platforms._any = rockspec_types.source
-rockspec_types.hooks.platforms._any = rockspec_types.hooks
-rockspec_types.test.platforms._any = rockspec_types.test
-
 --- Type check a rockspec table.
 -- Verify the correctness of elements from a 
 -- rockspec table, reporting on unknown fields and type
@@ -153,17 +151,19 @@ rockspec_types.test.platforms._any = rockspec_types.test
 -- succeeded, or nil and an error message if it failed.
 function type_rockspec.check(rockspec, globals)
    assert(type(rockspec) == "table")
-   if not rockspec.rockspec_format then
-      rockspec.rockspec_format = "1.0"
+   local version = rockspec.rockspec_format or "1.0"
+   local schema = rockspec_formats[version]
+   if not schema then
+      return nil, "unknown rockspec format " .. version
    end
-   local ok, err = type_check.check_undeclared_globals(globals, rockspec_types)
+   local ok, err = type_check.check_undeclared_globals(globals, schema)
    if ok then
-      ok, err = type_check.type_check_table(rockspec.rockspec_format, rockspec, rockspec_types, "")
+      ok, err = type_check.type_check_table(version, rockspec, schema, "")
    end
    if ok then
       return true
    end
-   return nil, err .. " (rockspec format " .. rockspec.rockspec_format .. ")"
+   return nil, err .. " (rockspec format " .. version .. ")"
 end
 
 return type_rockspec
diff --git a/src/luarocks/type_check.lua b/src/luarocks/type_check.lua
index ebd59f85..827c64b6 100644
--- a/src/luarocks/type_check.lua
+++ b/src/luarocks/type_check.lua
@@ -3,12 +3,86 @@ local type_check = {}
 
 local cfg = require("luarocks.core.cfg")
 local vers = require("luarocks.core.vers")
+local util = require("luarocks.util")
 local require = nil
 --------------------------------------------------------------------------------
 
-type_check.string_1 = { _type = "string" }
-type_check.number_1 = { _type = "number" }
-type_check.mandatory_string_1 = { _type = "string", _mandatory = true }
+type_check.MAGIC_PLATFORMS = {}
+
+do
+   local function fill_in_version(tbl, version)
+      for _, v in pairs(tbl) do
+         if type(v) == "table" then
+            if v._version == nil then
+               v._version = version
+            end
+            fill_in_version(v)
+         end
+      end
+   end
+   
+   local function expand_magic_platforms(tbl)
+      for k,v in pairs(tbl) do
+         if v == type_check.MAGIC_PLATFORMS then
+            tbl[k] = {
+               _any = util.deep_copy(tbl)
+            }
+            tbl[k]._any[v] = nil
+         elseif type(v) == "table" then
+            expand_magic_platforms(v)
+         end
+      end
+   end
+   
+   local function merge_under(dst, src)
+      for k, v in pairs(src) do
+         if dst[k] == nil then
+            if type(dst[k]) == "table" then
+               util.deep_merge(dst[k], v)
+            else
+               dst[k] = v
+            end
+         end
+      end
+   end
+   
+   -- Build a table of schemas.
+   -- @param versions a table where each key is a version number as a string,
+   -- and the value is a schema specification. Schema versions are considered
+   -- incremental: version "2.0" only needs to specify what's new/changed from
+   -- version "1.0".
+   function type_check.declare_schemas(versions)
+      local schemas = {}
+      local parent_version
+   
+      local version_list = {}
+      -- FIXME sorting lexicographically! "1.9" > "1.10"
+      for version, schema in util.sortedpairs(versions) do
+         table.insert(version_list, version)
+         if parent_version ~= nil then
+            local copy = util.deep_copy(schemas[parent_version])
+            util.deep_merge(copy, schema)
+            schemas[version] = copy
+         else
+            schemas[version] = schema
+         end
+         fill_in_version(schemas[version], version)
+         expand_magic_platforms(schemas[version])
+         parent_version = version
+      end
+      
+      -- Merge future versions as fallbacks under the old versions,
+      -- so that error messages can inform users when they try
+      -- to use new features without bumping rockspec_format in their rockspecs.
+      for i = #version_list, 2, -1 do
+         merge_under(schemas[version_list[i - 1]], schemas[version_list[i]])
+      end
+
+      return schemas
+   end
+end
+
+--------------------------------------------------------------------------------
 
 local function check_version(version, typetbl, context)
    local typetbl_version = typetbl._version or "1.0"
@@ -55,7 +129,7 @@ local function type_check_item(version, item, typetbl, context)
       if item_type ~= "string" then
          return nil, "Type mismatch on field "..context..": expected a string, got "..item_type
       end
-      local pattern = (typetbl._patterns and typetbl._patterns[version]) or typetbl._pattern
+      local pattern = typetbl._pattern
       if pattern then
          if not item:match("^"..pattern.."$") then
             local what = typetbl._name or ("'"..pattern.."'")
diff --git a/src/luarocks/util.lua b/src/luarocks/util.lua
index cd946451..f5506f53 100644
--- a/src/luarocks/util.lua
+++ b/src/luarocks/util.lua
@@ -15,6 +15,7 @@ util.keys = core.keys
 util.printerr = core.printerr
 util.sortedpairs = core.sortedpairs
 util.warning = core.warning
+util.deep_merge = core.deep_merge
 
 local unpack = unpack or table.unpack
 
@@ -479,4 +480,16 @@ function util.split_namespace(ns_name)
    return ns_name
 end
 
+function util.deep_copy(tbl)
+   local copy = {}
+   for k, v in pairs(tbl) do
+      if type(v) == "table" then
+         copy[k] = util.deep_copy(v)
+      else
+         copy[k] = v
+      end
+   end
+   return copy
+end
+
 return util