LACE: Start normalising error paths more aggressively

11 files changed, 151 insertions, 37 deletions

diff --git a/Makefile b/Makefile
index d6daa7b..6e16973 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@
 all: test
 
-MODULES := lace lace.lex lace.compiler lace.builtin lace.engine
+MODULES := lace lace.lex lace.compiler lace.builtin lace.engine lace.error
 LUA_VER := 5.1
 
 INST_BASE := /usr/local
diff --git a/lib/lace.lua b/lib/lace.lua
index e685a68..ba73af7 100644
--- a/lib/lace.lua
+++ b/lib/lace.lua
@@ -11,6 +11,7 @@ local lex = require "lace.lex"
 local compiler = require "lace.compiler"
 local builtin = require "lace.builtin"
 local engine = require "lace.engine"
+local error = require 'lace.error'
 
 local _VERSION = 0
 
@@ -21,6 +22,7 @@ return {
    compiler = compiler,
    builtin = builtin,
    engine = engine,
+   error = error,
    _VERSION = _VERSION,
    VERSION = VERSION,
 }
diff --git a/lib/lace/builtin.lua b/lib/lace/builtin.lua
index 9d7a735..03fe7bc 100644
--- a/lib/lace/builtin.lua
+++ b/lib/lace/builtin.lua
@@ -10,6 +10,7 @@
 local builtin = {}
 
 local engine = require "lace.engine"
+local err = require "lace.error"
 
 local function compiler()
    return require "lace.compiler"
@@ -77,10 +78,10 @@ end
 
 local function _return(compcontext, result, reason, ...)
    if result ~= "allow" and result ~= "deny" then
-      return compiler().error("Unknown result: " .. result, {1})
+      return err.error("Unknown result: " .. result, {1})
    end
    if type(reason) ~= "string" then
-      return compiler().error("Expected reason, got nothing")
+      return err.error("Expected reason, got nothing")
    end
 
    local cond = {...}
@@ -103,20 +104,20 @@ builtin.deny = _return
 function builtin.default(compcontext, def, result, reason, unwanted)
    assert(def == "default", "Somehow, builtin.default got something odd")
    if type(result) ~= "string" then
-      return compiler().error("Expected result, got nothing")
+      return err.error("Expected result, got nothing")
    end
    if result ~= "allow" and result ~= "deny" then
-      return compiler().error("Result wasn't allow or deny", {2})
+      return err.error("Result wasn't allow or deny", {2})
    end
    if type(reason) ~= "string" then
       reason = "Default behaviour"
    end
    if unwanted ~= nil then
-      return compiler().error("Unexpected additional content", {4})
+      return err.error("Unexpected additional content", {4})
    end
 
    if compcontext._lace.default then
-      return compiler().error("Cannot change the default")
+      return err.error("Cannot change the default")
    end
    
    local uncond, last = unconditional_result, last_result
@@ -133,10 +134,10 @@ end
 
 local function _compile_any_all_of(compcontext, mtype, first, second, ...)
    if type(first) ~= "string" then
-      return compiler().error("Expected at least two names, got none")
+      return err.error("Expected at least two names, got none")
    end
    if type(second) ~= "string" then
-      return compiler().error("Expected at least two names, only got one")
+      return err.error("Expected at least two names, only got one")
    end
 
    return {
@@ -163,20 +164,20 @@ end
 
 function builtin.define(compcontext, define, name, controltype, ...)
    if type(name) ~= "string" then
-      return compiler().error("Expected name, got nothing")
+      return err.error("Expected name, got nothing")
    end
 
    if name == "" or name:sub(1,1) == "!" then
-      return compiler().error("Bad name for definition", {2})
+      return err.error("Bad name for definition", {2})
    end
 
    if type(controltype) ~= "string" then
-      return compiler().error("Expected control type, got nothing")
+      return err.error("Expected control type, got nothing")
    end
    
    local controlfn = _controlfn(compcontext, controltype)
    if not controlfn then
-      return compiler().error("Unknown control type: " .. controltype, {3})
+      return err.error("Unknown control type: " .. controltype, {3})
    end
 
    local ctrltab, msg = controlfn(compcontext, controltype, ...)
@@ -224,7 +225,7 @@ function builtin.include(comp_context, cmd, file, ...)
    local conds = {...}
    
    if type(file) ~= "string" then
-      return compiler().error("No file named for inclusion")
+      return err.error("No file named for inclusion")
    end
 
    local loader = compiler().internal_loader(comp_context)
@@ -246,7 +247,7 @@ function builtin.include(comp_context, cmd, file, ...)
    -- Okay, the file is present, let's parse it.
    local ruleset, msg = compiler().internal_compile(comp_context, real, content, true)
    if type(ruleset) ~= "table" then
-      return false, type(msg) == "table" and msg or compiler().error(msg)
+      return false, msg
    end
    
    -- Okay, we parsed, so build the runtime
diff --git a/lib/lace/compiler.lua b/lib/lace/compiler.lua
index d3a393d..4fd975b 100644
--- a/lib/lace/compiler.lua
+++ b/lib/lace/compiler.lua
@@ -9,17 +9,14 @@
 
 local lex = require "lace.lex"
 local builtin = require "lace.builtin"
-
-local function _error(str, words)
-   return false, { msg = str, words = words }
-end
+local err = require "lace.error"
 
 local function _fake_loader(ctx, name)
-   return _error("Ruleset not found: " .. name, {1})
+   return err.error("Ruleset not found: " .. name, {1})
 end
 
 local function _fake_command(ctx)
-   return _error("Command is disabled by context")
+   return err.error("Command is disabled by context")
 end
 
 local function _loader(ctx)
@@ -42,7 +39,7 @@ local function _command(ctx, name)
    return cfn
 end
 
-local function _normalise_error(ctx, err)
+local function _normalise_error(ctx, err, offset)
    -- For now, just return the error
    return err
 end
@@ -58,7 +55,7 @@ local function compile_one_line(compcontext, line)
    local cmdname = line.content[1].str
    local cmdfn = _command(compcontext, cmdname)
    if type(cmdfn) ~= "function" then
-      return _error("Unknown command: " .. cmdname, {1})
+      return err.error("Unknown command: " .. cmdname, {1})
    end
 
    local args = {}
@@ -78,7 +75,7 @@ local function internal_compile_ruleset(compcontext, sourcename, content, suppre
       -- No content supplied, try and load it.
       sourcename, content = _loader(compcontext)(compcontext, sourcename)
       if type(sourcename) ~= "string" then
-	 return false, _normalise_error(compcontext, content)
+	 return false, content
       end
    end
 
@@ -108,7 +105,7 @@ local function internal_compile_ruleset(compcontext, sourcename, content, suppre
 	 _setposition(compcontext, ruleset, i)
 	 local rule, msg = compile_one_line(compcontext, line)
 	 if type(rule) ~= "table" then
-	    return rule, (rule == nil) and msg or _normalise_error(compcontext, msg)
+	    return rule, msg
 	 end
 	 rule.linenr = i
 	 ruleset.rules[#ruleset.rules+1] = rule
@@ -127,7 +124,7 @@ local function internal_compile_ruleset(compcontext, sourcename, content, suppre
    --   There's no unconditional result and no default, fake up a default and
    --     then use it.
    if not suppress_default and not uncond and not result then
-      return false, "No result set whatsoever"
+      return false, { msg = "No result set whatsoever", words = {} }
    end
 
    if not suppress_default and not uncond then
@@ -152,10 +149,11 @@ local function compile_ruleset(ctx, src, cnt)
       return nil, ret
    end
 
+   assert((ret) or (type(msg) == "table"), "Prenormalised error! " .. tostring(msg))
+
    if type(msg) == "table" then
-      -- TODO: Extract position information etc from error and
-      -- formulate a gorgeous multiline error message.
-      msg = msg.msg or "Empty error"
+      assert(type(msg.msg) == "string", "No error message")
+      msg = msg.msg
    end
 
    return ret, msg
@@ -165,5 +163,4 @@ return {
    internal_loader = _loader,
    internal_compile = internal_compile_ruleset,
    compile = compile_ruleset,
-   error = _error,
 }
diff --git a/lib/lace/error.lua b/lib/lace/error.lua
new file mode 100644
index 0000000..d52da1d
--- /dev/null
+++ b/lib/lace/error.lua
@@ -0,0 +1,16 @@
+-- lib/lace/error.lua
+--
+-- Lua Access Control Engine - Error management
+--
+-- Copyright 2012 Daniel Silverstone <dsilvers@digital-scurf.org>
+--
+-- For licence terms, see COPYING
+--
+
+local function _error(str, words)
+   return false, { msg = str, words = words or {} }
+end
+
+return {
+   error = _error
+}
\ No newline at end of file
diff --git a/test/test-lace.builtin.lua b/test/test-lace.builtin.lua
index 913843b..270567c 100644
--- a/test/test-lace.builtin.lua
+++ b/test/test-lace.builtin.lua
@@ -16,6 +16,13 @@ local engine = require 'lace.engine'
 
 local testnames = {}
 
+local real_assert = assert
+local total_asserts = 0
+local function assert(...)
+   real_assert(...)
+   total_asserts = total_asserts + 1
+end
+
 local function add_test(suite, name, value)
    rawset(suite, name, value)
    testnames[#testnames+1] = name
@@ -619,6 +626,6 @@ for _, testname in ipairs(testnames) do
    end
 end
 
-print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " OK")
+print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " [" .. tostring(total_asserts) .. "] OK")
 
 os.exit(count_ok == #testnames and 0 or 1)
diff --git a/test/test-lace.compiler.lua b/test/test-lace.compiler.lua
index 42471f6..c8a41ae 100644
--- a/test/test-lace.compiler.lua
+++ b/test/test-lace.compiler.lua
@@ -12,9 +12,17 @@
 local luacov = require 'luacov'
 
 local compiler = require 'lace.compiler'
+local err = require 'lace.error'
 
 local testnames = {}
 
+local real_assert = assert
+local total_asserts = 0
+local function assert(...)
+   real_assert(...)
+   total_asserts = total_asserts + 1
+end
+
 local function add_test(suite, name, value)
    rawset(suite, name, value)
    testnames[#testnames+1] = name
@@ -122,7 +130,7 @@ local comp_context = {
 		  end
 		  local fh = io.open("test/test-lace.compile-" .. name .. ".rules", "r")
 		  if not fh then
-		     return compiler.error("LOADER: Unknown: " .. name, {1})
+		     return err.error("LOADER: Unknown: " .. name, {1})
 		  end
 		  local content = fh:read("*a")
 		  fh:close()
@@ -192,6 +200,6 @@ for _, testname in ipairs(testnames) do
    end
 end
 
-print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " OK")
+print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " [" .. tostring(total_asserts) .. "] OK")
 
 os.exit(count_ok == #testnames and 0 or 1)
diff --git a/test/test-lace.engine.lua b/test/test-lace.engine.lua
index 8cdd3ea..f8faaf8 100644
--- a/test/test-lace.engine.lua
+++ b/test/test-lace.engine.lua
@@ -15,6 +15,13 @@ local lace = require 'lace'
 
 local testnames = {}
 
+local real_assert = assert
+local total_asserts = 0
+local function assert(...)
+   real_assert(...)
+   total_asserts = total_asserts + 1
+end
+
 local function add_test(suite, name, value)
    rawset(suite, name, value)
    testnames[#testnames+1] = name
@@ -109,7 +116,7 @@ local comp_context = {
 		  end
 		  local fh = io.open("test/test-lace.engine-" .. name .. ".rules", "r")
 		  if not fh then
-		     return compiler.error("LOADER: Unknown: " .. name, {1})
+		     return lace.error.error("LOADER: Unknown: " .. name, {1})
 		  end
 		  local content = fh:read("*a")
 		  fh:close()
@@ -211,6 +218,6 @@ for _, testname in ipairs(testnames) do
    end
 end
 
-print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " OK")
+print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " [" .. tostring(total_asserts) .. "] OK")
 
 os.exit(count_ok == #testnames and 0 or 1)
diff --git a/test/test-lace.error.lua b/test/test-lace.error.lua
new file mode 100644
index 0000000..4c15acd
--- /dev/null
+++ b/test/test-lace.error.lua
@@ -0,0 +1,55 @@
+-- test/test-lace.lua
+--
+-- Lua Access Control Engine -- Tests for the Lace error module
+--
+-- Copyright 2012 Daniel Silverstone <dsilvers@digital-scurf.org>
+--
+-- For Licence terms, see COPYING
+--
+
+-- Step one, start coverage
+
+local luacov = require 'luacov'
+
+local error = require 'lace.error'
+
+local testnames = {}
+
+local real_assert = assert
+local total_asserts = 0
+local function assert(...)
+   real_assert(...)
+   total_asserts = total_asserts + 1
+end
+
+local function add_test(suite, name, value)
+   rawset(suite, name, value)
+   testnames[#testnames+1] = name
+end
+
+local suite = setmetatable({}, {__newindex = add_test})
+
+function suite.error_formation()
+   local words = {}
+   local ret1, ret2 = error.error("msg", words)
+   assert(ret1 == false, "First return of error() should be false")
+   assert(type(ret2) == "table", "Second return of error() should be a table")
+   assert(ret2.msg == "msg", "Message should be passed through")
+   assert(ret2.words == words, "Words should be passed through")
+end
+
+local count_ok = 0
+for _, testname in ipairs(testnames) do
+--   print("Run: " .. testname)
+   local ok, err = xpcall(suite[testname], debug.traceback)
+   if not ok then
+      print(err)
+      print()
+   else
+      count_ok = count_ok + 1
+   end
+end
+
+print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " [" .. tostring(total_asserts) .. "] OK")
+
+os.exit(count_ok == #testnames and 0 or 1)
diff --git a/test/test-lace.lex.lua b/test/test-lace.lex.lua
index 543309a..25b2444 100644
--- a/test/test-lace.lex.lua
+++ b/test/test-lace.lex.lua
@@ -15,6 +15,14 @@ local lex = require 'lace.lex'
 
 local testnames = {}
 
+local real_assert = assert
+local total_asserts = 0
+local function assert(...)
+   local retval = real_assert(...)
+   total_asserts = total_asserts + 1
+   return retval
+end
+
 local function add_test(suite, name, value)
    rawset(suite, name, value)
    testnames[#testnames+1] = name
@@ -272,6 +280,6 @@ for _, testname in ipairs(testnames) do
    end
 end
 
-print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " OK")
+print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " [" .. tostring(total_asserts) .. "] OK")
 
 os.exit(count_ok == #testnames and 0 or 1)
diff --git a/test/test-lace.lua b/test/test-lace.lua
index d8bdb0d..1a80eca 100644
--- a/test/test-lace.lua
+++ b/test/test-lace.lua
@@ -16,9 +16,18 @@ local lex = require 'lace.lex'
 local compiler = require 'lace.compiler'
 local builtin = require 'lace.builtin'
 local engine = require 'lace.engine'
+local error = require 'lace.error'
 
 local testnames = {}
 
+local real_assert = assert
+local total_asserts = 0
+local function assert(...)
+   local retval = real_assert(...)
+   total_asserts = total_asserts + 1
+   return retval
+end
+
 local function add_test(suite, name, value)
    rawset(suite, name, value)
    testnames[#testnames+1] = name
@@ -42,6 +51,10 @@ function suite.engine_passed()
    assert(lace.engine == engine, "Lace's engine entry is not lace.engine")
 end
 
+function suite.error_passed()
+   assert(lace.error == error, "Lace's error entry is not lace.error")
+end
+
 local count_ok = 0
 for _, testname in ipairs(testnames) do
 --   print("Run: " .. testname)
@@ -54,6 +67,6 @@ for _, testname in ipairs(testnames) do
    end
 end
 
-print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " OK")
+print(tostring(count_ok) .. "/" .. tostring(#testnames) .. " [" .. tostring(total_asserts) .. "] OK")
 
 os.exit(count_ok == #testnames and 0 or 1)