Mr. Stradivarius has uploaded a new change for review.
https://gerrit.wikimedia.org/r/145547
Change subject: Improve error messages in mw.html
......................................................................
Improve error messages in mw.html
This sets the location of errors raised by mw.html.lua to the
calling module, rather than inside mw.html.lua itself. Some errors also
now display the type and position in the argument list of the argument
that caused the error, via the new verifyString function. This should
make it easier for module writers to locate errors in their code.
Also, the format of the error changes has been switched from
"Error message: Explanation" to "error message (explanation)", in
line with other Scribunto library error messages. Type errors
generated with verifyString have been changed to "bad argument #n
in 'html:foo' (expected bar, got baz)", in line with the checktype
function in the libraryUtil library.
Change-Id: If9cf9a52bd4b1bb42cc7f9f1f1096828710cbc52
---
M engines/LuaCommon/lualib/mw.html.lua
M tests/engines/LuaCommon/HtmlLibraryTests.lua
2 files changed, 73 insertions(+), 54 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Scribunto
refs/changes/47/145547/1
diff --git a/engines/LuaCommon/lualib/mw.html.lua
b/engines/LuaCommon/lualib/mw.html.lua
index 7fd503b..b458e5b 100644
--- a/engines/LuaCommon/lualib/mw.html.lua
+++ b/engines/LuaCommon/lualib/mw.html.lua
@@ -52,6 +52,35 @@
return table.concat( ret )
end
+-- Verify that an argument is a string. Number arguments and nil arguments
+-- can also be used if their flags are set.
+--
+-- @param name Name of the calling function
+-- @param argIdx Position of the argument in the function parameter list
+-- @param arg
+-- @param numOk Set to true if numbers are allowed
+-- @param nilOk Set to true if nil is allowed
+local function verifyString( name, argIdx, arg, numOk, nilOk )
+ if type( arg ) == 'string'
+ or type( arg ) == 'number' and numOk
+ or arg == nil and nilOk
+ then
+ return arg
+ end
+ local msg
+ if nilOk and numOk then
+ msg = "bad argument #%d to '%s' (expected string, number, or
nil; got %s)"
+ elseif numOk then
+ msg = "bad argument #%d to '%s' (expected string or number, got
%s)"
+ elseif nilOk then
+ msg = "bad argument #%d to '%s' (expected string or nil, got
%s)"
+ else
+ msg = "bad argument #%d to '%s' (expected string, got %s)"
+ end
+ msg = string.format( msg, argIdx, name, type( arg ) )
+ error( msg, 3 )
+end
+
-- Get an attribute table (name, value) and its index
--
-- @param name
@@ -85,7 +114,7 @@
return string.gsub( s, '[<>&"]', htmlencodeMap )
end
- local function cssEncode( s )
+local function cssEncode( s )
-- XXX: I'm not sure this character set is complete.
return mw.ustring.gsub( s, '[;:%z\1-\31\127-\244\143\191\191]',
function ( m )
return string.format( '\\%X ', mw.ustring.codepoint( m ) )
@@ -141,7 +170,7 @@
-- @param builder
methodtable.node = function( t, builder )
if t.selfClosing then
- error( "Self-closing tags can't have child nodes" )
+ error( "self-closing tags can't have child nodes", 2 )
end
if builder then
@@ -154,11 +183,8 @@
methodtable.wikitext = function( t, ... )
local vals = {...}
for i = 1, #vals do
- if type( vals[i] ) ~= 'string' and type( vals[i] ) ~= 'number'
then
- error( 'Invalid wikitext given: Must be either a string
or a number' )
- end
-
- t:node( vals[i] )
+ local val = verifyString( 'html:wikitext', i, vals[i], true,
true )
+ t:node( val )
end
return t
end
@@ -200,7 +226,10 @@
methodtable.attr = function( t, name, val )
if type( name ) == 'table' then
if val ~= nil then
- error( 'If a key->value table is given as first
parameter, value must be left empty' )
+ error(
+ 'if a key->value table is given as first
parameter, the second parameter must be left empty',
+ 2
+ )
end
local callForTable = function()
@@ -210,18 +239,14 @@
end
if not pcall( callForTable ) then
- error( 'Invalid table given: Must be name (string)
value (string|number) pairs' )
+ error( 'invalid table given (keys must be strings, and
values must be strings or numbers)', 2 )
end
return t
end
- if type( name ) ~= 'string' then
- error( 'Invalid name given: The name must be a string' )
- end
- if val ~= nil and type( val ) ~= 'string' and type( val ) ~= 'number'
then
- error( 'Invalid value given: The value must be either a string
or a number' )
- end
+ name = verifyString( 'html:attr', 1, name )
+ val = verifyString( 'html:attr', 2, val, true, true )
-- if caller sets the style attribute explicitly, then replace all
styles
-- previously added with css() and cssText()
@@ -231,7 +256,8 @@
end
if not isValidAttributeName( name ) then
- error( "Invalid attribute name: " .. name )
+ local msg = string.format( "invalid attribute name '%s'", name )
+ error( msg, 2 )
end
local attr, i = getAttr( t, name )
@@ -257,9 +283,7 @@
return t
end
- if type( class ) ~= 'string' and type( class ) ~= 'number' then
- error( 'Invalid class given: The name must be either a string
or a number' )
- end
+ class = verifyString( 'html:addClass', 1, class, true, true )
local attr = getAttr( t, 'class' )
if attr then
@@ -278,7 +302,10 @@
methodtable.css = function( t, name, val )
if type( name ) == 'table' then
if val ~= nil then
- error( 'If a key->value table is given as first
parameter, value must be left empty' )
+ error(
+ 'if a key->value table is given as first
parameter, the second parameter must be left empty',
+ 2
+ )
end
local callForTable = function()
@@ -288,18 +315,14 @@
end
if not pcall( callForTable ) then
- error( 'Invalid table given: Must be name
(string|number) value (string|number) pairs' )
+ error( 'invalid table given (keys and values must be
either strings or numbers)', 2 )
end
return t
end
- if type( name ) ~= 'string' and type( name ) ~= 'number' then
- error( 'Invalid CSS given: The name must be either a string or
a number' )
- end
- if val ~= nil and type( val ) ~= 'string' and type( val ) ~= 'number'
then
- error( 'Invalid CSS given: The value must be either a string or
a number' )
- end
+ name = verifyString( 'html:css', 1, name, true, false )
+ val = verifyString( 'html:css', 2, val, true, true )
for i, prop in ipairs( t.styles ) do
if prop.name == name then
@@ -325,10 +348,7 @@
-- @param css
methodtable.cssText = function( t, css )
if css ~= nil then
- if type( css ) ~= 'string' and type( css ) ~= 'number' then
- error( 'Invalid CSS given: Must be either a string or a
number' )
- end
-
+ css = verifyString( 'html:cssText', 1, css, true, true )
table.insert( t.styles, css )
end
return t
@@ -356,12 +376,11 @@
-- @param args
function HtmlBuilder.create( tagName, args )
if tagName ~= nil then
- if type( tagName ) ~= 'string' then
- error( "Tag name must be a string" )
- end
+ tagName = verifyString( 'mw.html.create', 1, tagName, false,
true )
if tagName ~= '' and not isValidTag( tagName ) then
- error( "Invalid tag name: " .. tagName )
+ local msg = string.format( "invalid tag name '%s'",
tagName )
+ error( msg, 2 )
end
end
diff --git a/tests/engines/LuaCommon/HtmlLibraryTests.lua
b/tests/engines/LuaCommon/HtmlLibraryTests.lua
index de69828..428dae2 100644
--- a/tests/engines/LuaCommon/HtmlLibraryTests.lua
+++ b/tests/engines/LuaCommon/HtmlLibraryTests.lua
@@ -118,11 +118,11 @@
},
{ name = 'mw.html.create (invalid tag 1)', func = mw.html.create,
type='ToString',
args = { '$$$$' },
- expect = 'Invalid tag name: $$$$'
+ expect = "invalid tag name '$$$$'"
},
{ name = 'mw.html.create (invalid tag 2)', func = mw.html.create,
type='ToString',
args = { {} },
- expect = 'Tag name must be a string'
+ expect = "bad argument #1 to 'mw.html.create' (expected string or
nil, got table)"
},
{ name = 'mw.html.wikitext', func = testHelper, type='ToString',
args = { getEmptyTestDiv(), 'wikitext', 'Plain text' },
@@ -130,7 +130,7 @@
},
{ name = 'mw.html.wikitext (invalid input)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'wikitext', 'Plain text', {} },
- expect = 'Invalid wikitext given: Must be either a string or a number'
+ expect = "bad argument #2 to 'html:wikitext' (expected string,
number, or nil; got table)"
},
{ name = 'mw.html.newline', func = testHelper, type='ToString',
args = { getEmptyTestDiv(), 'newline' },
@@ -159,35 +159,35 @@
},
{ name = 'mw.html.attr (invalid name 1)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'attr', 123, 'bar' },
- expect = 'Invalid name given: The name must be a string'
+ expect = "bad argument #1 to 'html:attr' (expected string, got
number)"
},
{ name = 'mw.html.attr (invalid name 2)', func = testHelper,
args = { getEmptyTestDiv(), 'attr', '§§§§', 'foo' },
- expect = 'Invalid attribute name: §§§§'
+ expect = "invalid attribute name '§§§§'"
},
{ name = 'mw.html.attr (table no value)', func = testHelper,
args = { getEmptyTestDiv(), 'attr', { foo = 'bar' }, 'foo' },
- expect = 'If a key->value table is given as first parameter, value
must be left empty'
+ expect = 'if a key->value table is given as first parameter, the
second parameter must be left empty'
},
{ name = 'mw.html.attr (invalid value)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'attr', 'foo', true },
- expect = 'Invalid value given: The value must be either a string or a
number'
+ expect = "bad argument #2 to 'html:attr' (expected string, number, or
nil; got boolean)"
},
{ name = 'mw.html.attr (invalid table 1)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'attr', { foo = {} } },
- expect = 'Invalid table given: Must be name (string) value
(string|number) pairs'
+ expect = 'invalid table given (keys must be strings, and values must
be strings or numbers)'
},
{ name = 'mw.html.attr (invalid table 2)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'attr', { 1, 2 ,3 } },
- expect = 'Invalid table given: Must be name (string) value
(string|number) pairs'
+ expect = 'invalid table given (keys must be strings, and values must
be strings or numbers)'
},
{ name = 'mw.html.attr (invalid table 3)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'attr', { foo = 'bar', blah = true } },
- expect = 'Invalid table given: Must be name (string) value
(string|number) pairs'
+ expect = 'invalid table given (keys must be strings, and values must
be strings or numbers)'
},
{ name = 'mw.html.attr (invalid table 4)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'attr', { [{}] = 'foo' } },
- expect = 'Invalid table given: Must be name (string) value
(string|number) pairs'
+ expect = 'invalid table given (keys must be strings, and values must
be strings or numbers)'
},
{ name = 'mw.html.getAttr (nil)', func = testHelper,
args = { getEmptyTestDiv(), 'getAttr', 'foo' },
@@ -203,7 +203,7 @@
},
{ name = 'mw.html.addClass (invalid value)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'addClass', {} },
- expect = 'Invalid class given: The name must be either a string or a
number'
+ expect = "bad argument #1 to 'html:addClass' (expected string,
number, or nil; got table)"
},
{ name = 'mw.html.css', func = testHelper, type='ToString',
args = { getEmptyTestDiv(), 'css', 'foo', 'bar' },
@@ -219,15 +219,15 @@
},
{ name = 'mw.html.css (invalid name 1)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'css', function() end, 'bar' },
- expect = 'Invalid CSS given: The name must be either a string or a
number'
+ expect = "bad argument #1 to 'html:css' (expected string or number,
got function)"
},
{ name = 'mw.html.css (table no value)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'css', {}, 'bar' },
- expect = 'If a key->value table is given as first parameter, value
must be left empty'
+ expect = 'if a key->value table is given as first parameter, the
second parameter must be left empty'
},
{ name = 'mw.html.css (invalid value)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'css', 'foo', {} },
- expect = 'Invalid CSS given: The value must be either a string or a
number'
+ expect = "bad argument #2 to 'html:css' (expected string, number, or
nil; got table)"
},
{ name = 'mw.html.css (table)', func = testHelper, type='ToString',
args = { getEmptyTestDiv(), 'css', testAttrs },
@@ -235,7 +235,7 @@
},
{ name = 'mw.html.css (invalid table)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'css', { foo = 'bar', ab = true } },
- expect = 'Invalid table given: Must be name (string|number) value
(string|number) pairs'
+ expect = 'invalid table given (keys and values must be either strings
or numbers)'
},
{ name = 'mw.html.cssText', func = testHelper, type='ToString',
args = { getEmptyTestDiv(), 'cssText', 'Unit tests, ftw' },
@@ -247,7 +247,7 @@
},
{ name = 'mw.html.cssText (invalid value)', func = testHelper,
type='ToString',
args = { getEmptyTestDiv(), 'cssText', {} },
- expect = 'Invalid CSS given: Must be either a string or a number'
+ expect = "bad argument #1 to 'html:cssText' (expected string, number,
or nil; got table)"
},
{ name = 'mw.html attribute escaping (value with double quotes)', func
= testHelper, type='ToString',
args = { getEmptyTestDiv(), 'attr', 'foo', 'ble"rgh' },
@@ -296,10 +296,10 @@
expect = { '<div><br /></div>' }
},
{ name = 'mw.html.node (append to self closing)', func =
testNodeAppendToSelfClosing, type='ToString',
- expect = "Self-closing tags can't have child nodes"
+ expect = "self-closing tags can't have child nodes"
},
{ name = 'mw.html.wikitext (append to self closing)', func =
testWikitextAppendToSelfClosing, type='ToString',
- expect = "Self-closing tags can't have child nodes"
+ expect = "self-closing tags can't have child nodes"
},
{ name = 'mw.html.tag.node (using allDone)', func = testTagNodeAllDone,
type='ToString',
expect = { '<div><p><div></div></p></div>' }
--
To view, visit https://gerrit.wikimedia.org/r/145547
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If9cf9a52bd4b1bb42cc7f9f1f1096828710cbc52
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Scribunto
Gerrit-Branch: master
Gerrit-Owner: Mr. Stradivarius <misterst...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
