From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 999654696C3 for ; Wed, 15 Apr 2020 23:58:09 +0300 (MSK) Date: Wed, 15 Apr 2020 23:51:02 +0300 From: Igor Munkin Message-ID: <20200415205102.GF8314@tarantool.org> References: <56290abaaa1850a223eac0fa7165bcb9f890501d.1586849129.git.sergeyb@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <56290abaaa1850a223eac0fa7165bcb9f890501d.1586849129.git.sergeyb@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 1/6] Fix luacheck warnings in src/lua/ List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Bronnikov Cc: o.piskunov@tarantool.org, tarantool-patches@dev.tarantool.org Sergey, Thanks for the patch! On 14.04.20, Sergey Bronnikov wrote: > Many warnings fixed with help from Vladislav Shpilevoy. > > Closes #4681 > > Reviewed-by: Vladislav Shpilevoy > --- > > src/lua/argparse.lua | 6 ++-- > src/lua/buffer.lua | 4 +-- > src/lua/clock.lua | 2 +- > src/lua/crypto.lua | 4 +-- > src/lua/csv.lua | 5 ++- > src/lua/digest.lua | 2 +- > src/lua/env.lua | 2 +- > src/lua/fio.lua | 25 +++++++------- > src/lua/httpc.lua | 3 -- > src/lua/init.lua | 7 ++-- > src/lua/msgpackffi.lua | 16 +++------ > src/lua/socket.lua | 65 ++++++++++++++++++------------------- > src/lua/string.lua | 1 - > src/lua/swim.lua | 10 +++--- > src/lua/tap.lua | 74 ++++++++++++++++++++---------------------- > src/lua/trigger.lua | 3 -- > 16 files changed, 105 insertions(+), 124 deletions(-) > > diff --git a/src/lua/argparse.lua b/src/lua/argparse.lua > index faa0ae130..f58985425 100644 > --- a/src/lua/argparse.lua > +++ b/src/lua/argparse.lua > @@ -90,8 +90,8 @@ local function convert_parameter(name, convert_from, convert_to) > return convert_from > end > > -local function parameters_parse(t_in, options) > - local t_out, t_in = {}, t_in or {} > +local function parameters_parse(t__in, options) > + local t_out, t_in = {}, t__in or {} I guess you can just give t__in a proper name, e.g. args, params, etc. > > -- Prepare a lookup table for options. An option name -> a > -- type name to convert a parameter into or true (which means > diff --git a/src/lua/crypto.lua b/src/lua/crypto.lua > index bf3064c70..c0eb0d303 100644 > --- a/src/lua/crypto.lua > +++ b/src/lua/crypto.lua > @@ -428,7 +428,7 @@ local public_methods = { > } > > local module_mt = { > - __serialize = function(s) > + __serialize = function() > return public_methods > end, > __index = public_methods I see no reasons to leave other W212[unused argument self] occurences. Here is a diff: ================================================================================ diff --git a/src/lua/crypto.lua b/src/lua/crypto.lua index c0eb0d303..6a7ee53f0 100644 --- a/src/lua/crypto.lua +++ b/src/lua/crypto.lua @@ -318,7 +318,7 @@ for class, digest in pairs(digests) do digest_api[class] = setmetatable({ new = function () return digest_new(digest) end }, { - __call = function (self, str) + __call = function (_, str) if type(str) ~= 'string' then error("Usage: digest."..class.."(string)") end @@ -332,7 +332,7 @@ for class, digest in pairs(digests) do end digest_api = setmetatable(digest_api, - {__index = function(self, digest) + {__index = function(_, digest) return error('Digest method "' .. digest .. '" is not supported') end }) @@ -341,7 +341,7 @@ for class, digest in pairs(hmacs) do hmac_api[class] = setmetatable({ new = function (key) return hmac_new(digest, key) end }, { - __call = function (self, key, str) + __call = function (_, key, str) if type(str) ~= 'string' then error("Usage: hmac."..class.."(key, string)") end @@ -356,12 +356,12 @@ for class, digest in pairs(hmacs) do if type(str) ~= 'string' then error("Usage: hmac."..class.."_hex(key, string)") end - return string.hex(hmac_api[class](key, str)) + return hmac_api[class](key, str):hex() end end hmac_api = setmetatable(hmac_api, - {__index = function(self, digest) + {__index = function(_, digest) return error('HMAC method "' .. digest .. '" is not supported') end }) @@ -384,12 +384,12 @@ local crypto_dirs = { } local algo_api_mt = { - __index = function(self, mode) + __index = function(_, mode) error('Cipher mode ' .. mode .. ' is not supported') end } local crypto_api_mt = { - __index = function(self, cipher) + __index = function(_, cipher) return error('Cipher method "' .. cipher .. '" is not supported') end } @@ -408,7 +408,7 @@ for algo_name, algo_value in pairs(crypto_algos) do dir_value) end }, { - __call = function(self, str, key, iv) + __call = function(_, str, key, iv) local ctx = crypto_stream_new(algo_value, mode_value, key, iv, dir_value) local res = ctx:update(str) ================================================================================ > diff --git a/src/lua/digest.lua b/src/lua/digest.lua > index 6ed91cfa2..7f1aea8d0 100644 > --- a/src/lua/digest.lua > +++ b/src/lua/digest.lua > @@ -246,7 +246,7 @@ local m = { > end > } > > -for digest, name in pairs(digest_shortcuts) do > +for digest, _ in pairs(digest_shortcuts) do > m[digest] = function (str) > return crypto.digest[digest](str) > end Fixed the remaining errors: ================================================================================ diff --git a/src/lua/digest.lua b/src/lua/digest.lua index 7f1aea8d0..6175228ab 100644 --- a/src/lua/digest.lua +++ b/src/lua/digest.lua @@ -91,7 +91,7 @@ PMurHash = { } setmetatable(PMurHash, { - __call = function(self, str) + __call = function(_, str) if type(str) ~= 'string' then error("Usage: digest.murhash(string)") end @@ -134,7 +134,7 @@ CRC32 = { } setmetatable(CRC32, { - __call = function(self, str) + __call = function(_, str) if type(str) ~= 'string' then error("Usage digest.crc32(string)") end ================================================================================ Fixed the remaining errors in src/lua/errno.lua: ================================================================================ diff --git a/src/lua/errno.lua b/src/lua/errno.lua index db800ce30..9b8dd518a 100644 --- a/src/lua/errno.lua +++ b/src/lua/errno.lua @@ -17,5 +17,5 @@ return setmetatable({ }, { __index = errno_list, __newindex = function() error("Can't create new errno constants") end, - __call = function(self, ...) return ffi.errno(...) end + __call = function(_, ...) return ffi.errno(...) end }) ================================================================================ > diff --git a/src/lua/fio.lua b/src/lua/fio.lua > index 83fddaa0a..b2b4b4c77 100644 > --- a/src/lua/fio.lua > +++ b/src/lua/fio.lua > @@ -329,7 +329,7 @@ fio.abspath = function(path) > error("Usage: fio.abspath(path)") > end > path = path It is a confusing noop, please drop it. > - local joined_path = '' > + local joined_path > local path_tab = {} > if string.sub(path, 1, 1) == '/' then > joined_path = path > @@ -407,20 +407,20 @@ fio.rmtree = function(path) > if type(path) ~= 'string' then > error("Usage: fio.rmtree(path)") > end > - local status, err > + local status This value is unused (and the corresponding warning is reported). Consider the changes below: ================================================================================ diff --git a/src/lua/fio.lua b/src/lua/fio.lua index b2b4b4c77..e271eccf6 100644 --- a/src/lua/fio.lua +++ b/src/lua/fio.lua @@ -407,7 +407,6 @@ fio.rmtree = function(path) if type(path) ~= 'string' then error("Usage: fio.rmtree(path)") end - local status path = fio.abspath(path) local ls, err = fio.listdir(path) if err ~= nil then @@ -427,7 +426,8 @@ fio.rmtree = function(path) end end end - status, err = fio.rmdir(path) + local _ + _, err = fio.rmdir(path) if err ~= nil then return false, string.format("failed to remove %s: %s", path, err) end ================================================================================ > path = fio.abspath(path) > local ls, err = fio.listdir(path) > if err ~= nil then > return nil, err > end > - for i, f in ipairs(ls) do > + for _, f in ipairs(ls) do > local tmppath = fio.pathjoin(path, f) > local st = fio.lstat(tmppath) > if st then > if st:is_dir() then > - st, err = fio.rmtree(tmppath) > + _, err = fio.rmtree(tmppath) > else > - st, err = fio.unlink(tmppath) > + _, err = fio.unlink(tmppath) > end > if err ~= nil then > return nil, err > @@ -471,10 +471,10 @@ fio.copytree = function(from, to) > if reason ~= nil then > return false, reason > end > - for i, f in ipairs(ls) do > + for _, f in ipairs(ls) do > local ffrom = fio.pathjoin(from, f) > local fto = fio.pathjoin(to, f) > - local st = fio.lstat(ffrom) > + st = fio.lstat(ffrom) > if st and st:is_dir() then > status, reason = fio.copytree(ffrom, fto) This value is unused (and the corresponding warning is reported). Consider the changes below: ================================================================================ diff --git a/src/lua/fio.lua b/src/lua/fio.lua index b2b4b4c77..5fc47b8fc 100644 --- a/src/lua/fio.lua +++ b/src/lua/fio.lua @@ -453,7 +453,6 @@ fio.copytree = function(from, to) if type(from) ~= 'string' or type(to) ~= 'string' then error('Usage: fio.copytree(from, to)') end - local status, reason local st = fio.stat(from) if not st then return false, string.format("Directory %s does not exist", from) @@ -467,7 +466,7 @@ fio.copytree = function(from, to) end -- create tree of destination - status, reason = fio.mktree(to) + local _, reason = fio.mktree(to) if reason ~= nil then return false, reason end @@ -476,13 +475,13 @@ fio.copytree = function(from, to) local fto = fio.pathjoin(to, f) st = fio.lstat(ffrom) if st and st:is_dir() then - status, reason = fio.copytree(ffrom, fto) + _, reason = fio.copytree(ffrom, fto) if reason ~= nil then return false, reason end end if st:is_reg() then - status, reason = fio.copyfile(ffrom, fto) + _, reason = fio.copyfile(ffrom, fto) if reason ~= nil then return false, reason end @@ -493,7 +492,7 @@ fio.copytree = function(from, to) if reason ~= nil then return false, reason end - status, reason = fio.symlink(link_to, fto) + _, reason = fio.symlink(link_to, fto) if reason ~= nil then return false, "can't create symlink in place of existing file "..fto end ================================================================================ > if reason ~= nil then > diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua > index f01ffaef0..9105c3f23 100644 > --- a/src/lua/msgpackffi.lua > +++ b/src/lua/msgpackffi.lua > @@ -504,7 +497,7 @@ end > > local ext_decoder = { > -- MP_UNKNOWN_EXTENSION > - [0] = function(data, len) error("unsupported extension type") end, > + [0] = function() error("unsupported extension type") end, > -- MP_DECIMAL > [1] = function(data, len) local num = ffi.new("decimal_t") builtin.decimal_unpack(data, len, num) return num end, Again another suppressed warning, that can be simply fixed: ================================================================================ diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua index 9105c3f23..786118d2d 100644 --- a/src/lua/msgpackffi.lua +++ b/src/lua/msgpackffi.lua @@ -499,9 +499,17 @@ local ext_decoder = { -- MP_UNKNOWN_EXTENSION [0] = function() error("unsupported extension type") end, -- MP_DECIMAL - [1] = function(data, len) local num = ffi.new("decimal_t") builtin.decimal_unpack(data, len, num) return num end, + [1] = function(data, len) + local num = ffi.new("decimal_t") + builtin.decimal_unpack(data, len, num) + return num + end, -- MP_UUID - [2] = function(data, len) local uuid = ffi.new("struct tt_uuid") builtin.uuid_unpack(data, len, uuid) return uuid end, + [2] = function(data, len) + local uuid = ffi.new("struct tt_uuid") + builtin.uuid_unpack(data, len, uuid) + return uuid + end, } local function decode_ext(data) ================================================================================ > -- MP_UUID > diff --git a/src/lua/socket.lua b/src/lua/socket.lua > index a334ad45b..317acfe8e 100644 > --- a/src/lua/socket.lua > +++ b/src/lua/socket.lua Fixed the remaining errors: ================================================================================ diff --git a/src/lua/socket.lua b/src/lua/socket.lua index 317acfe8e..75a4e8fb5 100644 --- a/src/lua/socket.lua +++ b/src/lua/socket.lua @@ -1577,7 +1577,7 @@ return setmetatable({ iowait = internal.iowait, internal = internal, }, { - __call = function(self, ...) return socket_new(...) end; + __call = function(_, ...) return socket_new(...) end; __index = { tcp = lsocket_tcp; connect = lsocket_connect; ================================================================================ > diff --git a/src/lua/tap.lua b/src/lua/tap.lua > index 94b080d5a..094eb883f 100644 > --- a/src/lua/tap.lua > +++ b/src/lua/tap.lua > @@ -190,38 +186,38 @@ local function isboolean(test, v, message, extra) > return is(test, type(v), 'boolean', message, extra) > end > I don't get the difference between routine and , but you changed parameter name only in one of them. Please, choose the one naming and adjust everything considering it. > -local function isfunction(test, v, message, extra) > - return is(test, type(v), 'function', message, extra) > +local function isfunction(testcase, v, message, extra) > + return is(testcase, type(v), 'function', message, extra) > end > > -local function isudata(test, v, utype, message, extra) > +local function isudata(testcase, v, utype, message, extra) > extra = extra or {} > extra.expected = 'userdata<'..utype..'>' > if type(v) == 'userdata' then > extra.got = 'userdata<'..getmetatable(v)..'>' > - return ok(test, getmetatable(v) == utype, message, extra) > + return ok(testcase, getmetatable(v) == utype, message, extra) > else > extra.got = type(v) > - return fail(test, message, extra) > + return fail(testcase, message, extra) > end > end > > -local function iscdata(test, v, ctype, message, extra) > +local function iscdata(testcase, v, ctype, message, extra) > extra = extra or {} > extra.expected = ffi.typeof(ctype) > if type(v) == 'cdata' then > extra.got = ffi.typeof(v) > - return ok(test, ffi.istype(ctype, v), message, extra) > + return ok(testcase, ffi.istype(ctype, v), message, extra) > else > extra.got = type(v) > - return fail(test, message, extra) > + return fail(testcase, message, extra) > end > end > > local test_mt > local function test(parent, name, fun, ...) > local level = parent ~= nil and parent.level + 1 or 0 > - local test = setmetatable({ > + local testcase = setmetatable({ > parent = parent; > name = name; > level = level; > -- > 2.23.0 > > > -- > sergeyb@ -- Best regards, IM