Re: [Tarantool-patches] [PATCH 1/6] Fix luacheck warnings in src/lua/

[Igor Munkin <imun@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 <v.shpilevoy@tarantool.org>
> ---
> 
>  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

<snipped>

> 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

<snipped>

> @@ -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)

================================================================================

<snipped>

> 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

================================================================================

<snipped>

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

<snipped>

> @@ -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

<snipped>

> 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

<snipped>

> @@ -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

<snipped>

> 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

<snipped>

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;

================================================================================

<snipped>

> 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

<snipped>

> @@ -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 <isfunction> routine and <isboolean>,
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;

<snipped>

> -- 
> 2.23.0
> 
> 
> -- 
> sergeyb@

-- 
Best regards,
IM