From e55ff19c961b799774303d1c38a581693af289a2 Mon Sep 17 00:00:00 2001 From: Carl Smedstad Date: Tue, 16 Mar 2021 03:23:38 +0100 Subject: Run Luacheck in CI & fix reported errors (#1285) Tried to do this as non-intrusively as possible, mostly by ignoring the issues in-line. Set the option unused_secondaries to false as suggested by @hishamhm. This makes named but ununsed return values, that serves a documenting purpose, allowed. --- .luacheckrc | 2 ++ .travis.yml | 18 ++++++++++++++---- src/luarocks/build.lua | 2 +- src/luarocks/build/builtin.lua | 2 +- src/luarocks/cmd.lua | 2 +- src/luarocks/cmd/build.lua | 2 +- src/luarocks/cmd/doc.lua | 2 +- src/luarocks/cmd/install.lua | 3 +-- src/luarocks/cmd/make.lua | 7 ++++--- src/luarocks/cmd/pack.lua | 1 - src/luarocks/cmd/purge.lua | 2 ++ src/luarocks/cmd/remove.lua | 2 ++ src/luarocks/cmd/which.lua | 2 +- src/luarocks/core/cfg.lua | 4 ++-- src/luarocks/download.lua | 4 ++-- src/luarocks/fs.lua | 2 ++ src/luarocks/fs/lua.lua | 8 ++++---- src/luarocks/fs/win32.lua | 5 ++++- src/luarocks/fs/win32/tools.lua | 6 +++--- src/luarocks/loader.lua | 6 ++++-- src/luarocks/manif/writer.lua | 4 ++-- src/luarocks/repos.lua | 5 ++--- src/luarocks/tools/zip.lua | 2 -- 23 files changed, 56 insertions(+), 37 deletions(-) diff --git a/.luacheckrc b/.luacheckrc index b37bdc4a..98e05c23 100644 --- a/.luacheckrc +++ b/.luacheckrc @@ -28,3 +28,5 @@ ignore = { include_files = { "src/luarocks/**/*.lua" } + +unused_secondaries = false diff --git a/.travis.yml b/.travis.yml index f5b6bb37..b1872483 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,11 +17,14 @@ smoke_script: &smoke_script - export BRANCH=$(if [ "$TRAVIS_PULL_REQUEST" == "false" ]; then echo $TRAVIS_BRANCH; else echo $TRAVIS_PULL_REQUEST_BRANCH; fi) - ./makedist $BRANCH - ./smoke_test.sh luarocks-$BRANCH.tar.gz - + +lint_script: &lint_script + - luacheck . + unit_script: &unit_script - busted -o htest --exclude-tags=git,integration --verbose -Xhelper "lua_dir=$PWD/lua_install,travis" - busted -o htest --exclude-tags=git,integration --verbose -Xhelper "lua_dir=$PWD/lua_install,travis,env=full" - + integration_script: &integration_script - lua -v - if [ "$TRAVIS_OS_NAME" = "linux" ]; then shellcheck ./configure; fi @@ -30,7 +33,7 @@ integration_script: &integration_script - ./smoke_test.sh luarocks-dev.tar.gz - busted -o htest --exclude-tags=gpg,git,unit --verbose -Xhelper "lua_dir=$PWD/lua_install,travis" - busted -o htest --exclude-tags=gpg,git,unit --verbose -Xhelper "lua_dir=$PWD/lua_install,travis,env=full" - + jobs: include: # Smoke tests for release branches @@ -47,6 +50,12 @@ jobs: language: generic env: - LUA="luajit=2.1" + # Lint with Luacheck + - stage: Lint with Luacheck + script: *lint_script + os: linux + env: + - LUA="lua=5.3" # Unit tests for linux - stage: Unit on Linux script: *unit_script @@ -139,7 +148,7 @@ jobs: env: - LUA="luajit=2.1" language: generic - + before_install: - if [ ! -f lua_install/bin/luarocks ]; then pip install hererocks; fi - if [ ! -f lua_install/bin/luarocks ]; then hererocks lua_install -r^ --$LUA; fi @@ -148,6 +157,7 @@ before_install: install: - if [ ! -f lua_install/bin/busted ]; then luarocks install busted; fi - if [ ! -f lua_install/bin/luacov ]; then luarocks install cluacov; fi + - if [ ! -f lua_install/bin/luacheck ]; then luarocks install luacheck; fi - luarocks install busted-htest after_success: diff --git a/src/luarocks/build.lua b/src/luarocks/build.lua index 0e8e7f58..ae529856 100644 --- a/src/luarocks/build.lua +++ b/src/luarocks/build.lua @@ -94,7 +94,7 @@ local function check_macosx_deployment_target(rockspec) local version = util.popen_read("sw_vers -productVersion") if version:match("^%d+%.%d+%.%d+$") or version:match("^%d+%.%d+$") then if vers.compare_versions(target, version) then - return nil, ("This rock requires Mac OSX %s, and you are running %s."):format(targetversion, version) + return nil, ("This rock requires Mac OSX %s, and you are running %s."):format(target, version) end end patch_variable("CC") diff --git a/src/luarocks/build/builtin.lua b/src/luarocks/build/builtin.lua index 70651ce8..6241c718 100644 --- a/src/luarocks/build/builtin.lua +++ b/src/luarocks/build/builtin.lua @@ -252,7 +252,7 @@ function builtin.run(rockspec, no_install) end return execute(variables.LD.." "..variables.LIBFLAG, "-o", library, unpack(extras)) end - compile_static_library = function(library, objects, libraries, libdirs, name) + compile_static_library = function(library, objects, libraries, libdirs, name) -- luacheck: ignore 211 local ok = execute(variables.AR, "rc", library, unpack(objects)) if ok then ok = execute(variables.RANLIB, library) diff --git a/src/luarocks/cmd.lua b/src/luarocks/cmd.lua index c8bcb552..21f35e20 100644 --- a/src/luarocks/cmd.lua +++ b/src/luarocks/cmd.lua @@ -337,7 +337,7 @@ local function use_to_fix_location(key) return buf end -local function get_config_text(cfg) +local function get_config_text(cfg) -- luacheck: ignore 431 local deps = require("luarocks.deps") local libdir_ok = deps.check_lua_libdir(cfg.variables) diff --git a/src/luarocks/cmd/build.lua b/src/luarocks/cmd/build.lua index 31622f35..d1fccfdd 100644 --- a/src/luarocks/cmd/build.lua +++ b/src/luarocks/cmd/build.lua @@ -18,7 +18,7 @@ local make = require("luarocks.cmd.make") local cmd = require("luarocks.cmd") function cmd_build.add_to_parser(parser) - local cmd = parser:command("build", "Build and install a rock, compiling its C parts if any.\n".. + local cmd = parser:command("build", "Build and install a rock, compiling its C parts if any.\n".. -- luacheck: ignore 431 "If the sources contain a luarocks.lock file, uses it as an authoritative source for ".. "exact version of dependencies.\n".. "If no arguments are given, behaves as luarocks make.", util.see_also()) diff --git a/src/luarocks/cmd/doc.lua b/src/luarocks/cmd/doc.lua index 6345063d..2ab7e43c 100644 --- a/src/luarocks/cmd/doc.lua +++ b/src/luarocks/cmd/doc.lua @@ -66,7 +66,7 @@ function doc.command(args) util.printout(rock.." is not installed. Looking for it in the rocks servers...") return try_to_open_homepage(args.rock, args.namespace, args.version) end - name, version = iname, iversion + local name, version = iname, iversion local rockspec, err = fetch.load_local_rockspec(path.rockspec_file(name, version, repo)) if not rockspec then return nil,err end diff --git a/src/luarocks/cmd/install.lua b/src/luarocks/cmd/install.lua index 1fa15453..760649b0 100644 --- a/src/luarocks/cmd/install.lua +++ b/src/luarocks/cmd/install.lua @@ -14,10 +14,9 @@ local search = require("luarocks.search") local queries = require("luarocks.queries") local cfg = require("luarocks.core.cfg") local cmd = require("luarocks.cmd") -local dir = require("luarocks.dir") function install.add_to_parser(parser) - local cmd = parser:command("install", "Install a rock.", util.see_also()) + local cmd = parser:command("install", "Install a rock.", util.see_also()) -- luacheck: ignore 431 cmd:argument("rock", "The name of a rock to be fetched from a repository ".. "or a filename of a locally available rock.") diff --git a/src/luarocks/cmd/make.lua b/src/luarocks/cmd/make.lua index 78647b63..a72e6817 100644 --- a/src/luarocks/cmd/make.lua +++ b/src/luarocks/cmd/make.lua @@ -13,7 +13,6 @@ local fetch = require("luarocks.fetch") local pack = require("luarocks.pack") local remove = require("luarocks.remove") local deps = require("luarocks.deps") -local deplocks = require("luarocks.deplocks") local writer = require("luarocks.manif.writer") local cmd = require("luarocks.cmd") @@ -47,6 +46,7 @@ function make.cmd_options(parser) end function make.add_to_parser(parser) + -- luacheck: push ignore 431 local cmd = parser:command("make", [[ Builds sources in the current directory, but unlike "build", it does not fetch sources, etc., assuming everything is available in the current directory. If no @@ -65,6 +65,7 @@ authoritative source for exact version of dependencies. The --pin flag overrides and recreates this file scanning dependency based on ranges. ]], util.see_also()) :summary("Compile package in current directory using a rockspec.") + -- luacheck: pop cmd:argument("rockspec", "Rockspec for the rock to build.") :args("?") @@ -117,7 +118,7 @@ function make.command(args) return build.build_rockspec(rockspec, opts) elseif args.pack_binary_rock then return pack.pack_binary_rock(name, namespace, rockspec.version, args.sign, function() - local name, version = build.build_rockspec(rockspec, opts) + local name, version = build.build_rockspec(rockspec, opts) -- luacheck: ignore 431 if name and args.no_doc then util.remove_doc_dir(name, version) end @@ -128,7 +129,7 @@ function make.command(args) if not ok then return nil, err, cmd.errorcodes.PERMISSIONDENIED end ok, err = build.build_rockspec(rockspec, opts) if not ok then return nil, err end - local name, version = ok, err + local name, version = ok, err -- luacheck: ignore 421 if opts.build_only_deps then util.printout("Stopping after installing dependencies for " ..name.." "..version) diff --git a/src/luarocks/cmd/pack.lua b/src/luarocks/cmd/pack.lua index 9ceb2fa8..29a43e7b 100644 --- a/src/luarocks/cmd/pack.lua +++ b/src/luarocks/cmd/pack.lua @@ -5,7 +5,6 @@ local cmd_pack = {} local util = require("luarocks.util") local pack = require("luarocks.pack") -local signing = require("luarocks.signing") local queries = require("luarocks.queries") function cmd_pack.add_to_parser(parser) diff --git a/src/luarocks/cmd/purge.lua b/src/luarocks/cmd/purge.lua index b71baa7c..fa4be4d7 100644 --- a/src/luarocks/cmd/purge.lua +++ b/src/luarocks/cmd/purge.lua @@ -16,6 +16,7 @@ local queries = require("luarocks.queries") local cmd = require("luarocks.cmd") function purge.add_to_parser(parser) + -- luacheck: push ignore 431 local cmd = parser:command("purge", [[ This command removes rocks en masse from a given tree. By default, it removes all rocks from a tree. @@ -23,6 +24,7 @@ By default, it removes all rocks from a tree. The --tree option is mandatory: luarocks purge does not assume a default tree.]], util.see_also()) :summary("Remove all installed rocks from a tree.") + -- luacheck: pop cmd:flag("--old-versions", "Keep the highest-numbered version of each ".. "rock and remove the other ones. By default it only removes old ".. diff --git a/src/luarocks/cmd/remove.lua b/src/luarocks/cmd/remove.lua index cede148d..46f3e4a6 100644 --- a/src/luarocks/cmd/remove.lua +++ b/src/luarocks/cmd/remove.lua @@ -15,6 +15,7 @@ local queries = require("luarocks.queries") local cmd = require("luarocks.cmd") function cmd_remove.add_to_parser(parser) + -- luacheck: push ignore 431 local cmd = parser:command("remove", [[ Uninstall a rock. @@ -23,6 +24,7 @@ Will only perform the removal if it does not break dependencies. To override this check and force the removal, use --force or --force-fast.]], util.see_also()) :summary("Uninstall a rock.") + -- luacheck: pop cmd:argument("rock", "Name of the rock to be uninstalled.") :action(util.namespaced_name_action) diff --git a/src/luarocks/cmd/which.lua b/src/luarocks/cmd/which.lua index 9a363e85..7503df00 100644 --- a/src/luarocks/cmd/which.lua +++ b/src/luarocks/cmd/which.lua @@ -32,7 +32,7 @@ function which_cmd.command(args) local modpath = args.modname:gsub("%.", "/") for _, v in ipairs({"path", "cpath"}) do for p in package[v]:gmatch("([^;]+)") do - local pathname = p:gsub("%?", modpath) + local pathname = p:gsub("%?", modpath) -- luacheck: ignore 421 if fs.exists(pathname) then util.printout(pathname) util.printout("(found directly via package." .. v .. " -- not installed as a rock?)") diff --git a/src/luarocks/core/cfg.lua b/src/luarocks/core/cfg.lua index 4ac5ee28..d65681c1 100644 --- a/src/luarocks/core/cfg.lua +++ b/src/luarocks/core/cfg.lua @@ -10,8 +10,8 @@ -- files. Run `luarocks` with no arguments to see the locations of -- these files in your platform. -local next, table, pairs, require, os, pcall, ipairs, package, tonumber, type, assert = - next, table, pairs, require, os, pcall, ipairs, package, tonumber, type, assert +local table, pairs, require, os, pcall, ipairs, package, type, assert = + table, pairs, require, os, pcall, ipairs, package, type, assert local util = require("luarocks.core.util") local persist = require("luarocks.core.persist") diff --git a/src/luarocks/download.lua b/src/luarocks/download.lua index 70570344..1246e498 100644 --- a/src/luarocks/download.lua +++ b/src/luarocks/download.lua @@ -32,8 +32,8 @@ function download.download(arch, name, namespace, version, all, check_lua_versio local has_result = false local all_ok = true local any_err = "" - for name, result in pairs(results) do - for version, items in pairs(result) do + for name, result in pairs(results) do -- luacheck: ignore 422 + for version, items in pairs(result) do -- luacheck: ignore 422 for _, item in ipairs(items) do -- Ignore provided rocks. if item.arch ~= "installed" then diff --git a/src/luarocks/fs.lua b/src/luarocks/fs.lua index bd1a96a2..ecfc5a28 100644 --- a/src/luarocks/fs.lua +++ b/src/luarocks/fs.lua @@ -29,6 +29,7 @@ do if old_popen or old_execute then return end old_popen = io.popen + -- luacheck: push globals io os io.popen = function(one, two) if two == nil then print("\nio.popen: ", one) @@ -49,6 +50,7 @@ do end return unpack(code, 1, code.n) end + -- luacheck: pop end end diff --git a/src/luarocks/fs/lua.lua b/src/luarocks/fs/lua.lua index b9b36447..83ffbd47 100644 --- a/src/luarocks/fs/lua.lua +++ b/src/luarocks/fs/lua.lua @@ -215,7 +215,7 @@ function fs_lua.modules(at) local modules = {} local is_duplicate = {} - for _, path in ipairs(paths) do + for _, path in ipairs(paths) do -- luacheck: ignore 421 local files = fs.list_dir(path) for _, filename in ipairs(files or {}) do if filename:match("%.[lL][uU][aA]$") then @@ -705,7 +705,7 @@ local redirect_protocols = { https = luasec_ok and https, } -local function request(url, method, http, loop_control) +local function request(url, method, http, loop_control) -- luacheck: ignore 431 local result = {} if cfg.verbose then @@ -805,7 +805,7 @@ end -- via the HTTP Last-Modified header if the full download is needed. -- @return (boolean | (nil, string, string?)): True if successful, or -- nil, error message and optionally HTTPS error in case of errors. -local function http_request(url, filename, http, cache) +local function http_request(url, filename, http, cache) -- luacheck: ignore 431 if cache then local status = read_timestamp(filename..".status") local timestamp = read_timestamp(filename..".timestamp") @@ -824,7 +824,7 @@ local function http_request(url, filename, http, cache) end end - local result, status, headers, err = request(url, "HEAD", http) + local result, status, headers, err = request(url, "HEAD", http) -- luacheck: ignore 421 if not result then return fail_with_status(filename, status, headers) end diff --git a/src/luarocks/fs/win32.lua b/src/luarocks/fs/win32.lua index 8ae62cf0..00dd0163 100644 --- a/src/luarocks/fs/win32.lua +++ b/src/luarocks/fs/win32.lua @@ -15,8 +15,11 @@ local util = require("luarocks.util") -- See http://lua-users.org/lists/lua-l/2013-11/msg00367.html local _prefix = "type NUL && " local _popen, _execute = io.popen, os.execute + +-- luacheck: push globals io os io.popen = function(cmd, ...) return _popen(_prefix..cmd, ...) end os.execute = function(cmd, ...) return _execute(_prefix..cmd, ...) end +-- luacheck: pop --- Annotate command string for quiet execution. -- @param cmd string: A command-line string. @@ -38,7 +41,7 @@ end local function split_root(pathname) local drive = "" local root = "" - local rest = "" + local rest local unquoted = pathname:match("^['\"](.*)['\"]$") if unquoted then diff --git a/src/luarocks/fs/win32/tools.lua b/src/luarocks/fs/win32/tools.lua index 57e8c80e..d6202ab9 100644 --- a/src/luarocks/fs/win32/tools.lua +++ b/src/luarocks/fs/win32/tools.lua @@ -21,11 +21,11 @@ function tools.command_at(directory, cmd, exit_on_error) if exit_on_error then op = " && " end - local cmd = "cd " .. fs.Q(directory) .. op .. cmd + local cmd_prefixed = "cd " .. fs.Q(directory) .. op .. cmd if drive then - cmd = drive .. " & " .. cmd + cmd_prefixed = drive .. " & " .. cmd_prefixed end - return cmd + return cmd_prefixed end --- Create a directory if it does not already exist. diff --git a/src/luarocks/loader.lua b/src/luarocks/loader.lua index 5aa84632..a3bd8f1b 100644 --- a/src/luarocks/loader.lua +++ b/src/luarocks/loader.lua @@ -1,10 +1,12 @@ - --- A module which installs a Lua package loader that is LuaRocks-aware. -- This loader uses dependency information from the LuaRocks tree to load -- correct versions of modules. It does this by constructing a "context" -- table in the environment, which records which versions of packages were -- used to load previous modules, so that the loader chooses versions -- that are declared to be compatible with the ones loaded earlier. + +-- luacheck: globals luarocks + local loaders = package.loaders or package.searchers local require, ipairs, table, type, next, tostring, error = require, ipairs, table, type, next, tostring, error @@ -24,7 +26,7 @@ end local path = require("luarocks.core.path") local manif = require("luarocks.core.manif") local vers = require("luarocks.core.vers") -local require = nil +local require = nil -- luacheck: ignore 411 -------------------------------------------------------------------------------- -- Workaround for wrappers produced by older versions of LuaRocks diff --git a/src/luarocks/manif/writer.lua b/src/luarocks/manif/writer.lua index 8c6e4505..8e07702c 100644 --- a/src/luarocks/manif/writer.lua +++ b/src/luarocks/manif/writer.lua @@ -30,7 +30,7 @@ local function store_package_items(storage, name, version, items) local package_identifier = name.."/"..version - for item_name, path in pairs(items) do + for item_name, path in pairs(items) do -- luacheck: ignore 431 if not storage[item_name] then storage[item_name] = {} end @@ -54,7 +54,7 @@ local function remove_package_items(storage, name, version, items) local package_identifier = name.."/"..version - for item_name, path in pairs(items) do + for item_name, path in pairs(items) do -- luacheck: ignore 431 local all_identifiers = storage[item_name] for i, identifier in ipairs(all_identifiers) do diff --git a/src/luarocks/repos.lua b/src/luarocks/repos.lua index b2a7d81a..4a78dfb2 100644 --- a/src/luarocks/repos.lua +++ b/src/luarocks/repos.lua @@ -9,9 +9,8 @@ local util = require("luarocks.util") local dir = require("luarocks.dir") local manif = require("luarocks.manif") local vers = require("luarocks.core.vers") -local E = {} -local unpack = unpack or table.unpack +local unpack = unpack or table.unpack -- luacheck: ignore 211 --- Get type and name of an item (a module or a command) provided by a file. -- @param deploy_type string: rock manifest subtree the file comes from ("bin", "lua", or "lib"). @@ -77,7 +76,7 @@ function repos.recurse_rock_manifest_entry(entry, action) for file, sub in pairs(tree) do local sub_path = (parent_path and (parent_path .. "/") or "") .. file - local ok, err + local ok, err -- luacheck: ignore 231 if type(sub) == "table" then ok, err = do_recurse_rock_manifest_entry(sub, sub_path) diff --git a/src/luarocks/tools/zip.lua b/src/luarocks/tools/zip.lua index fb27456d..37091185 100644 --- a/src/luarocks/tools/zip.lua +++ b/src/luarocks/tools/zip.lua @@ -10,8 +10,6 @@ local dir = require("luarocks.dir") local pack = table.pack or function(...) return { n = select("#", ...), ... } end -local stat_ok, stat = pcall(require, "posix.sys.stat") - local function shr(n, m) return math.floor(n / 2^m) end -- cgit v1.2.3-55-g6feb