From 43581963d8211d8e8e917a44062176349e4937b1 Mon Sep 17 00:00:00 2001 From: Jon Michael Aanes Date: Sat, 15 Jul 2017 20:43:25 +0200 Subject: [PATCH] The function body search were too restrictive and ignored OOP-style functions and unicode named functions, resulting in a crash. This has been fixed. The error message for that specific error has been made for descriptive. --- function.lua | 19 ++++++++++++++++--- test/test_function.lua | 37 +++++++++++++++++++++++-------------- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/function.lua b/function.lua index cb1d499..ec36f3e 100644 --- a/function.lua +++ b/function.lua @@ -55,6 +55,16 @@ end -- Constants +-- FUNCTION_DEFINITION_MATCH is a lua pattern, for finding a function definition. +-- NOTE: It will match malformed unicode sequences, and thus assumes that the +-- string checked against have been checked by the lua interpreter. +local FUNCTION_DEFINITION_MATCH = + '.*%f[%a_]function%f[^%a_]%s*' .. -- Look for the function keyword + '([a-zA-Z0-9\128-\255_.:]*)' .. -- Look for the function name, if any + '%s*(%([a-zA-Z0-9\128-\255_,. \t]*%))' .. -- Look for the function parameter list. + '[ \t]*(.+)[ \t]*' .. -- Look for the function body + 'end' -- Look for the end keyword + local NR_CHARS_IN_LONG_FUNCTION_BODY = 30 -------------------------------------------------------------------------------- @@ -139,10 +149,13 @@ local function get_function_paramlist_and_body (info) local end_line_index = get_line_index(str, info.lastlinedefined + 1) -- Now find the function parameters and the function body. - local function_params, function_body = str:sub(start_line_index, end_line_index):match('.*%f[%a_]function%f[^%a_]%s*[a-zA-Z0-9_.]*%s*(%([a-zA-Z0-9_,. \t]*%))[ \t]*(.+)[ \t]*end') - assert(type(function_params) == 'string' and type(function_body) == 'string') + local function_name, function_params, function_body = str:sub(start_line_index, end_line_index):match(FUNCTION_DEFINITION_MATCH) + -- TODO: Use function_name for something. + if type(function_params) ~= 'string' or type(function_body) ~= 'string' then + error(('[pretty.function/internal]: Could not find the function defined on lines %i-%i (indices %i-%i) for string:\n\n%s\n'):format(info.linedefined, info.lastlinedefined, start_line_index, end_line_index, str)) + end function_body = function_body:match('^%s*(.-)%s*$') - assert(type(function_params) == 'string' and type(function_body) == 'string') + assert(type(function_body) == 'string') -- And return them. return function_params, function_body end diff --git a/test/test_function.lua b/test/test_function.lua index f9a0678..4a2d5bb 100644 --- a/test/test_function.lua +++ b/test/test_function.lua @@ -34,7 +34,7 @@ local TEST_UPVALUE = 42 -- Basic inline functions format_test { - name = 'Basic function formatting', + name = 'Function basic formatting', adv_getlocal = true, single = true, input = function () end, @@ -74,7 +74,7 @@ format_test { } format_test { - name = 'Closures don\'t affect functions printed in single mode 1', + name = 'Function with upvalues don\'t affected by them in single mode 1', adv_getlocal = true, single = true, input = function () l = TEST_UPVALUE end, @@ -82,7 +82,7 @@ format_test { } format_test { - name = 'Closures don\'t affect functions printed in single mode 2', + name = 'Function with upvalues don\'t affected by them in single mode 2', adv_getlocal = true, single = true, input = function () TEST_UPVALUE = true end, @@ -256,20 +256,21 @@ format_test { -- Embedding functions loaded with loadstring format_test { + name = 'Embed functions loaded with loadstring', single = true, - name = 'It\'s possible to get loadstring functions whole', input = loadstring('return function (a, b) return a + b end')(), expect = 'function (a, b) return a + b end', } format_test { + name = 'Embed functions with extra whitespace stripped', single = true, - name = 'Whitespace is automatically stripped from loadstring functions', input = loadstring('return function (a, b)\n return a + b\nend')(), expect = 'function (a, b) return a + b end', } format_test { + name = 'Embed nested function, when they end on different lines', single = true, adv_getlocal = true, input = loadstring('return function () return function () end\nend')()(), @@ -277,7 +278,7 @@ format_test { } format_test { - name = 'When finding the correct function becomes too hard, just ignore it 1', + name = 'If embedding the correct function becomes too hard, ignore 1', single = true, adv_getlocal = true, input = loadstring('return function () return function () end end')(), @@ -285,7 +286,7 @@ format_test { } format_test { - name = 'When finding the correct function becomes too hard, just ignore it 2', + name = 'If embedding the correct function becomes too hard, ignore 2', single = true, adv_getlocal = true, input = loadstring('return function () return function () end end')()(), @@ -293,7 +294,7 @@ format_test { } format_test { - name = 'Can still find body when body contains the word "function"', + name = 'Embed functions which contains the word "function"', single = true, adv_getlocal = true, input = loadstring('return function () function_body() end')(), @@ -301,7 +302,7 @@ format_test { } format_test { - name = 'Can still find body when defined over objct', + name = 'Embed functions defined with OOP style', single = true, adv_getlocal = true, input = loadstring('local obj = {} \n function obj:a () return self end \n return obj.a')(), @@ -309,7 +310,7 @@ format_test { } format_test { - name = 'Can find correct body, when body is on the same line, with different arguments', + name = 'Embed correct function, when two are on the same line, but with different arguments 1', single = true, adv_getlocal = true, input = loadstring 'return { a = function (a) return a end, b = function (b) return b end }' ()['a'], @@ -317,7 +318,15 @@ format_test { } format_test { - name = 'Use dots when function body is somewhat long', + name = 'Embed correct function, when two are on the same line, but with different arguments 2', + single = true, + adv_getlocal = true, + input = loadstring 'return { a = function (a) return a end, b = function (b) return b end }' ()['b'], + expect = 'function (b) return b end', +} + +format_test { + name = 'If embedding a function would be too long, ignore', single = true, adv_getlocal = true, input = loadstring('return function () return 1234578901235789012357890 end')(), @@ -325,7 +334,7 @@ format_test { } format_test { - name = 'Use dots when function body includes newline', + name = 'If embedding a function would include newline, ignore', single = true, adv_getlocal = true, input = loadstring('return function () -- Hi\n return 1234 end')(), @@ -375,8 +384,8 @@ if HAS_UNICODE_IDEN then format_test { name = 'Align functions with unicode-named parameters nicely', adv_getlocal = true, - input = loadstring 'return {\nψ = function (ψ) return ψ end,\nb = function (b) return b end\n}' (), - expect = '{\n ψ = function (ψ) return ψ end\n b = function (b) return b end\n}', + input = loadstring 'return {\nfunction (ψ) return ψ end,\nfunction (b) return b end\n}' (), + expect = '{\n function (ψ) return ψ end\n function (b) return b end\n}', } end