From ce3ac84ed911a68b064d3cd9f9b8f4310eff3c1c Mon Sep 17 00:00:00 2001 From: Peter Melnichenko Date: Fri, 2 Feb 2018 19:40:49 +0300 Subject: [PATCH] Refactor core check modules * Move chstate methods for creating warnings into modules doing detection, to make them more isolated and easier to test. * Minor refactoring and renaming. * Fix a bug when reporting unused mutually recursive functions as unused values. --- CHANGELOG.md | 5 + install.lua | 54 +-- luacheck-dev-1.rockspec | 6 +- spec/check_spec.lua | 19 ++ spec/cli_spec.lua | 6 +- spec/linearize_spec.lua | 15 +- ...alyze_spec.lua => resolve_locals_spec.lua} | 41 +-- src/luacheck/check.lua | 315 ++++-------------- ...itespace.lua => detect_bad_whitespace.lua} | 16 +- src/luacheck/detect_cyclomatic_complexity.lua | 41 ++- src/luacheck/detect_globals.lua | 52 ++- src/luacheck/detect_uninit_access.lua | 20 +- src/luacheck/detect_unreachable_code.lua | 18 +- src/luacheck/detect_unused_locals.lua | 93 +++++- src/luacheck/detect_unused_rec_funcs.lua | 35 +- src/luacheck/inline_options.lua | 8 +- src/luacheck/linearize.lua | 110 ++++-- src/luacheck/name_functions.lua | 4 +- .../{analyze.lua => resolve_locals.lua} | 9 +- 19 files changed, 448 insertions(+), 419 deletions(-) rename spec/{analyze_spec.lua => resolve_locals_spec.lua} (78%) rename src/luacheck/{whitespace.lua => detect_bad_whitespace.lua} (59%) rename src/luacheck/{analyze.lua => resolve_locals.lua} (98%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02205f1b..febbcf8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ functions with complexity higher than a configurable limit; disabled by default (#141). +### Fixes + +* Fixed errors or incorrect reporting when unused mutually recursive functions + have other values assigned to their local variables. + ## 0.21.2 (2017-11-13) ### Fixes diff --git a/install.lua b/install.lua index b289eee6..70d43989 100755 --- a/install.lua +++ b/install.lua @@ -94,42 +94,42 @@ end print(" Installing luacheck modules into " .. luacheck_src_dir) mkdir(luacheck_lib_dir) -for _, filename in ipairs { - "main.lua", +for _, filename in ipairs({ "init.lua", + "argparse.lua", + "builtin_standards.lua", + "cache.lua", + "check.lua", "config.lua", - "linearize.lua", - "analyze.lua", "core_utils.lua", - "check.lua", - "parser.lua", - "lexer.lua", - "filter.lua", - "options.lua", - "inline_options.lua", - "builtin_standards.lua", - "love_standard.lua", - "ngx_standard.lua", + "detect_bad_whitespace.lua", + "detect_cyclomatic_complexity.lua", + "detect_globals.lua", + "detect_uninit_access.lua", + "detect_unreachable_code.lua", + "detect_unused_locals.lua", + "detect_unused_rec_funcs.lua", "expand_rockspec.lua", - "multithreading.lua", - "cache.lua", + "filter.lua", "format.lua", - "version.lua", "fs.lua", "globbing.lua", - "utils.lua", - "argparse.lua", - "whitespace.lua", - "detect_cyclomatic_complexity.lua", - "detect_globals.lua", - "standards.lua", - "lua_fs.lua", + "inline_options.lua", + "lexer.lua", "lfs_fs.lua", - "detect_unused_rec_funcs.lua", - "detect_unreachable_code.lua", - "detect_uninit_access.lua", + "linearize.lua", + "love_standard.lua", + "lua_fs.lua", + "main.lua", "name_functions.lua", - "detect_unused_locals.lua"} do + "multithreading.lua", + "ngx_standard.lua", + "options.lua", + "parser.lua", + "resolve_locals.lua", + "standards.lua", + "utils.lua", + "version.lua"}) do copy("src" .. dirsep .. "luacheck" .. dirsep .. filename, luacheck_lib_dir) end diff --git a/luacheck-dev-1.rockspec b/luacheck-dev-1.rockspec index 9472a331..0ca34e3f 100644 --- a/luacheck-dev-1.rockspec +++ b/luacheck-dev-1.rockspec @@ -19,13 +19,13 @@ build = { type = "builtin", modules = { luacheck = "src/luacheck/init.lua", - ["luacheck.analyze"] = "src/luacheck/analyze.lua", ["luacheck.argparse"] = "src/luacheck/argparse.lua", ["luacheck.builtin_standards"] = "src/luacheck/builtin_standards.lua", ["luacheck.cache"] = "src/luacheck/cache.lua", ["luacheck.check"] = "src/luacheck/check.lua", ["luacheck.config"] = "src/luacheck/config.lua", ["luacheck.core_utils"] = "src/luacheck/core_utils.lua", + ["luacheck.detect_bad_whitespace"] = "src/luacheck/detect_bad_whitespace.lua", ["luacheck.detect_cyclomatic_complexity"] = "src/luacheck/detect_cyclomatic_complexity.lua", ["luacheck.detect_globals"] = "src/luacheck/detect_globals.lua", ["luacheck.detect_uninit_access"] = "src/luacheck/detect_uninit_access.lua", @@ -49,10 +49,10 @@ build = { ["luacheck.ngx_standard"] = "src/luacheck/ngx_standard.lua", ["luacheck.options"] = "src/luacheck/options.lua", ["luacheck.parser"] = "src/luacheck/parser.lua", + ["luacheck.resolve_locals"] = "src/luacheck/resolve_locals.lua", ["luacheck.standards"] = "src/luacheck/standards.lua", ["luacheck.utils"] = "src/luacheck/utils.lua", - ["luacheck.version"] = "src/luacheck/version.lua", - ["luacheck.whitespace"] = "src/luacheck/whitespace.lua" + ["luacheck.version"] = "src/luacheck/version.lua" }, install = { bin = { diff --git a/spec/check_spec.lua b/spec/check_spec.lua index fa9c5ffa..ac65a008 100644 --- a/spec/check_spec.lua +++ b/spec/check_spec.lua @@ -250,6 +250,25 @@ end ]]) end) + it("detects unused mutually recursive functions as values", function() + assert.same({ + {code = "311", name = "odd", line = 5, column = 10, end_column = 12}, + {code = "311", name = "even", line = 9, column = 10, end_column = 13} + }, check[[ +local even = 2 +local odd = 3 +(...)(even, odd) + +function odd(x) + return x == 1 or even(x - 1) +end + +function even(x) + return x == 0 or odd(x - 1) or even(x) +end +]]) + end) + it("does not incorrectly detect unused recursive functions inside unused functions", function() assert.same({ {code = "211", name = "unused", func = true, line = 1, column = 16, end_column = 21} diff --git a/spec/cli_spec.lua b/spec/cli_spec.lua index 60fa6e1a..a66a8aef 100644 --- a/spec/cli_spec.lua +++ b/spec/cli_spec.lua @@ -915,7 +915,7 @@ spec/samples/bad_code.lua local A,B,C,D,E,F="package","711","helper","function","embrace","hepler";return {{{"112",A,1,1,7,[23]={A,"loaded",true}},{B,[3]=1,[4]=1,[5]=1,[27]=1,[29]="main_chunk"},{B,[3]=3,[4]=7,[5]=14,[27]=1,[28]=C,[29]=D},{"211",C,3,16,21,[10]=true},{"212","...",3,23,25},{B,[3]=7,[4]=1,[5]=8,[27]=2,[28]=E,[29]=D},{"111",E,7,10,16,[11]=true,[23]={E}},{"412","opt",8,10,12,7,18},{"113",F,9,11,16,[23]={F}}},{},{24,0,26,9,3,0,21,31,26,3,0},{[4]="comment"}} spec/samples/python_code.lua (%d+) -return {{{"011",[3]=1,[4]=6,[5]=15,[12]="expected '=' near '__future__'"}},{},{}} +return {{{"011",[3]=1,[4]=6,[5]=15,[12]="expected '=' near '__future__'"}},{},{},{}} ]]):gsub("[%[%]]", "%%%0"))) -- luacheck: pop @@ -934,10 +934,10 @@ return {{{"011",[3]=1,[4]=6,[5]=15,[12]="expected '=' near '__future__'"}},{},{} %s spec/samples/python_code.lua %s -return {{{"111", "global", 1, 1, [23]={"global"}}, {"321", "uninit", 6, 8}},{},{}} +return {{{"111", "global", 1, 1, [23]={"global"}}, {"321", "uninit", 6, 8}},{},{},{}} spec/samples/good_code.lua %s -return {{{"011",[3]=5,[4]=7,[12]="this code is actually bad"}},{},{}} +return {{{"011",[3]=5,[4]=7,[12]="this code is actually bad"}},{},{},{}} spec/samples/bad_code.lua %s return {{},{},{}}]]):format(version, python_mtime, good_mtime, tostring(tonumber(bad_mtime) - 1))) diff --git a/spec/linearize_spec.lua b/spec/linearize_spec.lua index d70b1137..2f101aa3 100644 --- a/spec/linearize_spec.lua +++ b/spec/linearize_spec.lua @@ -1,20 +1,11 @@ local linearize = require "luacheck.linearize" local parser = require "luacheck.parser" -local utils = require "luacheck.utils" - -local ChState = utils.class() - -function ChState.__init() end -function ChState.warn_redefined() end -function ChState.warn_global() end -function ChState.warn_unused_label() end -function ChState.warn_unbalanced() end -function ChState.warn_empty_block() end local function get_line_(src) local ast = parser.parse(src) - local chstate = ChState() - return linearize(chstate, ast) + local chstate = {ast = ast, warnings = {}} + linearize(chstate) + return chstate.main_line end local function get_line(src) diff --git a/spec/analyze_spec.lua b/spec/resolve_locals_spec.lua similarity index 78% rename from spec/analyze_spec.lua rename to spec/resolve_locals_spec.lua index 34a9f8b9..f2672ad2 100644 --- a/spec/analyze_spec.lua +++ b/spec/resolve_locals_spec.lua @@ -1,35 +1,6 @@ -local analyze = require "luacheck.analyze" local linearize = require "luacheck.linearize" local parser = require "luacheck.parser" -local utils = require "luacheck.utils" - -local ChState = utils.class() - -function ChState.__init() end -function ChState.warn_redefined() end -function ChState.warn_global() end -function ChState.warn_unused_label() end -function ChState.warn_unused_variable() end -function ChState.warn_unused_value() end -function ChState.warn_unset() end - -local function get_line_(src) - local ast = parser.parse(src) - local chstate = ChState() - return linearize(chstate, ast) -end - -local function get_line(src) - local ok, res = pcall(get_line_, src) - - if ok then - return res - elseif type(res) == "table" then - return nil - else - error(res, 0) - end -end +local resolve_locals = require "luacheck.resolve_locals" local function used_variables_to_string(item) local buf = {} @@ -49,12 +20,14 @@ local function used_variables_to_string(item) end local function get_used_variables_as_string(src) - local line = get_line(src) - analyze(line) + local ast = parser.parse(src) + local chstate = {ast = ast, warnings = {}} + linearize(chstate) + resolve_locals(chstate) local buf = {} - for _, item in ipairs(line.items) do + for _, item in ipairs(chstate.main_line.items) do if item.accesses and next(item.accesses) then assert.is_table(item.used_values) table.insert(buf, used_variables_to_string(item)) @@ -64,7 +37,7 @@ local function get_used_variables_as_string(src) return table.concat(buf, "\n") end -describe("analyze", function() +describe("resolve_locals", function() describe("when resolving values", function() it("resolves values in linear cases", function() assert.equal([[ diff --git a/src/luacheck/check.lua b/src/luacheck/check.lua index b918d833..f89e046b 100644 --- a/src/luacheck/check.lua +++ b/src/luacheck/check.lua @@ -1,284 +1,76 @@ -local parser = require "luacheck.parser" -local linearize = require "luacheck.linearize" -local analyze = require "luacheck.analyze" -local name_functions = require "luacheck.name_functions" -local inline_options = require "luacheck.inline_options" -local utils = require "luacheck.utils" -local check_whitespace = require "luacheck.whitespace" +local detect_bad_whitespace = require "luacheck.detect_bad_whitespace" +local detect_cyclomatic_complexity = require "luacheck.detect_cyclomatic_complexity" local detect_globals = require "luacheck.detect_globals" local detect_uninit_access = require "luacheck.detect_uninit_access" local detect_unreachable_code = require "luacheck.detect_unreachable_code" local detect_unused_locals = require "luacheck.detect_unused_locals" local detect_unused_rec_funcs = require "luacheck.detect_unused_rec_funcs" -local detect_cyclomatic_complexity = require "luacheck.detect_cyclomatic_complexity" - -local function is_secondary(value) - return value.secondaries and value.secondaries.used -end - -local ChState = utils.class() - -function ChState:__init() - self.warnings = {} -end - -function ChState:warn(warning, implicit_self) - if not warning.end_column then - warning.end_column = implicit_self and warning.column or (warning.column + #warning.name - 1) - end - - table.insert(self.warnings, warning) -end - -local action_codes = { - set = "1", - mutate = "2", - access = "3" -} - -local type_codes = { - var = "1", - func = "1", - arg = "2", - loop = "3", - loopi = "3" -} - --- `index` describes an indexing, where `index[1]` is a global node --- and other items describe keys: each one is a string node, "not_string", --- or "unknown". `node` is literal base node that's indexed. --- E.g. in `local a = table.a; a.b = "c"` `node` is `a` node of the second --- statement and `index` describes `table.a.b`. --- `index.previous_indexing_len` is optional length of prefix of `index` array representing last assignment --- in the aliasing chain, e.g. `2` in the previous example (because last indexing --- is `table.a`). -function ChState:warn_global(node, index, is_lhs, is_top_scope) - local global = index[1] - local action = is_lhs and (#index == 1 and "set" or "mutate") or "access" - - local indexing = {} - - for i, field in ipairs(index) do - if field == "unknown" then - indexing[i] = true - elseif field == "not_string" then - indexing[i] = false - else - indexing[i] = field[1] - end - end - - -- and filter out the warning if the base of last indexing is already - -- undefined and has been reported. - -- E.g. avoid useless warning in the second statement of `local t = tabell; t.concat(...)`. - self:warn({ - code = "11" .. action_codes[action], - name = global[1], - indexing = indexing, - previous_indexing_len = index.previous_indexing_len, - line = node.location.line, - column = node.location.column, - end_column = node.location.column + #node[1] - 1, - top = is_top_scope and (action == "set") or nil, - indirect = node ~= global or nil - }) -end - --- W12* (read-only global) and W131 (unused global) are patched in during filtering. - -function ChState:warn_unused_variable(value, recursive, self_recursive, useless) - self:warn({ - code = "21" .. type_codes[value.var.type], - name = value.var.name, - line = value.location.line, - column = value.location.column, - secondary = is_secondary(value) or nil, - func = (value.type == "func") or nil, - mutually_recursive = not self_recursive and recursive or nil, - recursive = self_recursive, - self = value.var.self, - useless = value.var.name == "_" and useless or nil - }, value.var.self) -end - -function ChState:warn_unset(var) - self:warn({ - code = "221", - name = var.name, - line = var.location.line, - column = var.location.column - }) -end - -function ChState:warn_unaccessed(var, mutated) - -- Mark as secondary if all assigned values are secondary. - -- It is guaranteed that there are at least two values. - local secondary = true - - for _, value in ipairs(var.values) do - if not value.empty and not is_secondary(value) then - secondary = nil - break - end - end - - self:warn({ - code = "2" .. (mutated and "4" or "3") .. type_codes[var.type], - name = var.name, - line = var.location.line, - column = var.location.column, - secondary = secondary - }, var.self) -end - -function ChState:warn_unused_value(value, mutated, overwriting_node) - self:warn({ - code = "3" .. (mutated and "3" or "1") .. type_codes[value.type], - name = value.var.name, - overwritten_line = overwriting_node and overwriting_node.location.line, - overwritten_column = overwriting_node and overwriting_node.location.column, - line = value.location.line, - column = value.location.column, - secondary = is_secondary(value) or nil, - }, value.type == "arg" and value.var.self) -end - -function ChState:warn_unused_field_value(node, overwriting_node) - self:warn({ - code = "314", - field = node.field, - index = node.is_index, - overwritten_line = overwriting_node.location.line, - overwritten_column = overwriting_node.location.column, - line = node.location.line, - column = node.location.column, - end_column = node.location.column + #node.first_token - 1 - }) -end - -function ChState:warn_uninit(node, mutation) - self:warn({ - code = mutation and "341" or "321", - name = node[1], - line = node.location.line, - column = node.location.column - }) -end - -function ChState:warn_redefined(var, prev_var, same_scope) - if var.name ~= "..." then - self:warn({ - code = "4" .. (same_scope and "1" or (var.line == prev_var.line and "2" or "3")) .. type_codes[prev_var.type], - name = var.name, - line = var.location.line, - column = var.location.column, - self = var.self and prev_var.self, - prev_line = prev_var.location.line, - prev_column = prev_var.location.column - }, var.self) - end -end - -function ChState:warn_unreachable(location, unrepeatable, token) - self:warn({ - code = "51" .. (unrepeatable and "2" or "1"), - line = location.line, - column = location.column, - end_column = location.column + #token - 1 - }) -end - -function ChState:warn_unused_label(label) - self:warn({ - code = "521", - label = label.name, - line = label.location.line, - column = label.location.column, - end_column = label.end_column - }) -end - -function ChState:warn_unbalanced(location, shorter_lhs) - -- Location points to `=`. - self:warn({ - code = "53" .. (shorter_lhs and "1" or "2"), - line = location.line, - column = location.column, - end_column = location.column - }) -end - -function ChState:warn_empty_block(location, do_end) - -- Location points to `do`, `then` or `else`. - self:warn({ - code = "54" .. (do_end and "1" or "2"), - line = location.line, - column = location.column, - end_column = location.column + (do_end and 1 or 3) - }) -end +local inline_options = require "luacheck.inline_options" +local linearize = require "luacheck.linearize" +local name_functions = require "luacheck.name_functions" +local parser = require "luacheck.parser" +local resolve_locals = require "luacheck.resolve_locals" +local utils = require "luacheck.utils" -function ChState:warn_empty_statement(location) - self:warn({ +local function new_empty_statement_warning(location) + return { code = "551", line = location.line, column = location.column, end_column = location.column - }) -end - -function ChState:warn_cyclomatic_complexity(node, complexity) - local warning = { - code = "711", - complexity = complexity } +end - if node.location then - warning.line = node.location.line - warning.column = node.location.column - warning.end_column = node.location.column + #"function" - 1 - warning.function_name = node.name - warning.function_type = node[1][1] and node[1][1].implicit and "method" or "function" - else - warning.line = 1 - warning.column = 1 - warning.end_column = 1 - warning.function_type = "main_chunk" +local function detect_empty_statements(chstate) + for _, location in ipairs(chstate.useless_semicolons) do + table.insert(chstate.warnings, new_empty_statement_warning(location)) end - - self:warn(warning) end local function check_or_throw(src) - local ast, comments, code_lines, line_endings, semicolons = parser.parse(src) - name_functions(ast) - local chstate = ChState() - local line = linearize(chstate, ast) - - for _, location in ipairs(semicolons) do - chstate:warn_empty_statement(location) - end - - local lines = utils.split_lines(src) - local line_lengths = utils.map(function(s) return #s end, lines) - check_whitespace(chstate, lines, line_endings) - analyze(line) - - detect_globals(chstate, line) - detect_unused_locals(chstate, line) - detect_uninit_access(chstate, line) - detect_unreachable_code(chstate, line) - detect_unused_rec_funcs(chstate, line) - detect_cyclomatic_complexity(chstate, line) + local ast, comments, code_lines, line_endings, useless_semicolons = parser.parse(src) + + local chstate = { + ast = ast, + comments = comments, + code_lines = code_lines, + line_endings = line_endings, + useless_semicolons = useless_semicolons, + source_lines = utils.split_lines(src), + warnings = {} + } - local events, per_line_opts = inline_options.get_events(ast, comments, code_lines, chstate.warnings) - return {events = events, per_line_options = per_line_opts, line_lengths = line_lengths, line_endings = line_endings} + linearize(chstate) + name_functions(chstate) + resolve_locals(chstate) + + detect_bad_whitespace(chstate) + detect_cyclomatic_complexity(chstate) + detect_empty_statements(chstate) + detect_globals(chstate) + detect_uninit_access(chstate) + detect_unreachable_code(chstate) + detect_unused_locals(chstate) + detect_unused_rec_funcs(chstate) + + local events, per_line_options = inline_options.get_events(chstate) + + return { + events = events, + per_line_options = per_line_options, + line_lengths = utils.map(function(s) return #s end, chstate.source_lines), + line_endings = line_endings + } end --- Checks source. -- Returns a table with results, with the following fields: -- `events`: array of issues and inline option events (options, push, or pop). -- `per_line_options`: map from line numbers to arrays of inline option events. +-- `line_lengths`: map from line numbers to line lengths. +-- `line_endings`: map from line numbers to "comment", "string", or `nil` base on +-- whether the line ending is within a token. +-- If `events` array contains a syntax error, the other fields are empty tables. local function check(src) local ok, res = utils.try(check_or_throw, src) @@ -293,7 +85,12 @@ local function check(src) msg = res.err.msg } - return {events = {syntax_error}, per_line_options = {}, line_lengths = {}} + return { + events = {syntax_error}, + per_line_options = {}, + line_lengths = {}, + line_endings = {} + } else error(res, 0) end diff --git a/src/luacheck/whitespace.lua b/src/luacheck/detect_bad_whitespace.lua similarity index 59% rename from src/luacheck/whitespace.lua rename to src/luacheck/detect_bad_whitespace.lua index 1984cf33..97e9bda1 100644 --- a/src/luacheck/whitespace.lua +++ b/src/luacheck/detect_bad_whitespace.lua @@ -1,5 +1,5 @@ -local function check_whitespace(chstate, lines, line_endings) - for line_number, line in ipairs(lines) do +local function detect_bad_whitespace(chstate) + for line_number, line in ipairs(chstate.source_lines) do if line ~= "" then local from, to = line:find("%s+$") @@ -9,28 +9,28 @@ local function check_whitespace(chstate, lines, line_endings) if from == 1 then -- Line contains only whitespace (thus never considered "code"). code = "611" - elseif not line_endings[line_number] then + elseif not chstate.line_endings[line_number] then -- Trailing whitespace on code line or after long comment. code = "612" - elseif line_endings[line_number] == "string" then + elseif chstate.line_endings[line_number] == "string" then -- Trailing whitespace embedded in a string literal. code = "613" - elseif line_endings[line_number] == "comment" then + elseif chstate.line_endings[line_number] == "comment" then -- Trailing whitespace at the end of a line comment or inside long comment. code = "614" end - chstate:warn({code = code, line = line_number, column = from, end_column = to}) + table.insert(chstate.warnings, {code = code, line = line_number, column = from, end_column = to}) end from, to = line:find("^%s+") if from and to ~= #line and line:sub(1, to):find(" \t") then -- Inconsistent leading whitespace (SPACE followed by TAB). - chstate:warn({code = "621", line = line_number, column = from, end_column = to}) + table.insert(chstate.warnings, {code = "621", line = line_number, column = from, end_column = to}) end end end end -return check_whitespace +return detect_bad_whitespace diff --git a/src/luacheck/detect_cyclomatic_complexity.lua b/src/luacheck/detect_cyclomatic_complexity.lua index c1b30a19..facdac99 100644 --- a/src/luacheck/detect_cyclomatic_complexity.lua +++ b/src/luacheck/detect_cyclomatic_complexity.lua @@ -1,5 +1,27 @@ local utils = require "luacheck.utils" +local function new_cyclomatic_complexity_warning(node, complexity) + local warning = { + code = "711", + complexity = complexity + } + + if node.location then + warning.line = node.location.line + warning.column = node.location.column + warning.end_column = node.location.column + #"function" - 1 + warning.function_name = node.name + warning.function_type = node[1][1] and node[1][1].implicit and "method" or "function" + else + warning.line = 1 + warning.column = 1 + warning.end_column = 1 + warning.function_type = "main_chunk" + end + + return warning +end + local CyclomaticComplexityMetric = utils.class() function CyclomaticComplexityMetric:incr_decisions(count) @@ -108,23 +130,22 @@ function CyclomaticComplexityMetric:calc_stmts(stmts) end end --- Cyclomatic complexity of a function equals to the number of decision points plus 1 -function CyclomaticComplexityMetric:calculate(line) - -- assert(line.node.tag == "Function", line.node.tag) - - -- reset - self.count = 0 +-- Cyclomatic complexity of a function equals to the number of decision points plus 1. +function CyclomaticComplexityMetric:report(chstate, line) + self.count = 1 self:calc_stmts(line.node[2]) self:calc_items(line.items) + table.insert(chstate.warnings, new_cyclomatic_complexity_warning(line.node, self.count)) return self.count + 1 end -local function detect_cyclomatic_complexity(chstate, line) +local function detect_cyclomatic_complexity(chstate) local ccmetric = CyclomaticComplexityMetric() - chstate:warn_cyclomatic_complexity(line.node, ccmetric:calculate(line)) + ccmetric:report(chstate, chstate.main_line) - for _, subline in ipairs(line.lines) do - chstate:warn_cyclomatic_complexity(subline.node, ccmetric:calculate(subline)) + for _, nested_line in ipairs(chstate.main_line.lines) do + ccmetric:report(chstate, nested_line) end end + return detect_cyclomatic_complexity diff --git a/src/luacheck/detect_globals.lua b/src/luacheck/detect_globals.lua index 179d6a60..185cf856 100644 --- a/src/luacheck/detect_globals.lua +++ b/src/luacheck/detect_globals.lua @@ -1,5 +1,47 @@ local utils = require "luacheck.utils" +local action_codes = { + set = "1", + mutate = "2", + access = "3" +} + +-- `index` describes an indexing, where `index[1]` is a global node +-- and other items describe keys: each one is a string node, "not_string", +-- or "unknown". `node` is literal base node that's indexed. +-- E.g. in `local a = table.a; a.b = "c"` `node` is `a` node of the second +-- statement and `index` describes `table.a.b`. +-- `index.previous_indexing_len` is optional length of prefix of `index` array representing last assignment +-- in the aliasing chain, e.g. `2` in the previous example (because last indexing is `table.a`). +local function new_global_warning(node, index, is_lhs, is_top_scope) + local global = index[1] + local action = is_lhs and (#index == 1 and "set" or "mutate") or "access" + + local indexing = {} + + for i, field in ipairs(index) do + if field == "unknown" then + indexing[i] = true + elseif field == "not_string" then + indexing[i] = false + else + indexing[i] = field[1] + end + end + + return { + code = "11" .. action_codes[action], + name = global[1], + indexing = indexing, + previous_indexing_len = index.previous_indexing_len, + line = node.location.line, + column = node.location.column, + end_column = node.location.column + #node[1] - 1, + top = is_top_scope and (action == "set") or nil, + indirect = node ~= global or nil + } +end + local function resolved_to_index(resolution) return resolution ~= "unknown" and resolution ~= "not_string" and resolution.tag ~= "String" end @@ -129,7 +171,7 @@ local function detect_in_node(chstate, item, node, is_top_line, is_lhs) end if resolved_to_index(resolution) then - chstate:warn_global(node, resolution, is_lhs, is_top_line) + table.insert(chstate.warnings, new_global_warning(node, resolution, is_lhs, is_top_line)) end elseif node.tag ~= "Function" then for _, nested_node in ipairs(node) do @@ -161,12 +203,12 @@ local function detect_in_line(chstate, line, is_top_line) end end --- Detects assignments, field accesses and mutations of global variables, +-- Adds warnings for assignments, field accesses, and mutations of global variables, -- tracing through localizing assignments such as `local t = table`. -local function detect_globals(chstate, line) - detect_in_line(chstate, line, true) +local function detect_globals(chstate) + detect_in_line(chstate, chstate.main_line, true) - for _, nested_line in ipairs(line.lines) do + for _, nested_line in ipairs(chstate.main_line.lines) do detect_in_line(chstate, nested_line) end end diff --git a/src/luacheck/detect_uninit_access.lua b/src/luacheck/detect_uninit_access.lua index 0c3a0aa6..44e52536 100644 --- a/src/luacheck/detect_uninit_access.lua +++ b/src/luacheck/detect_uninit_access.lua @@ -1,3 +1,13 @@ +local function new_uninit_warning(node, is_mutation) + return { + code = is_mutation and "341" or "321", + name = node[1], + line = node.location.line, + column = node.location.column, + end_column = node.location.column + #node[1] - 1 + } +end + local function detect_uninit_access_in_line(chstate, line) for _, item in ipairs(line.items) do for _, action_key in ipairs({"accesses", "mutations"}) do @@ -23,7 +33,7 @@ local function detect_uninit_access_in_line(chstate, line) if all_possible_values_empty then for _, accessing_node in ipairs(accessing_nodes) do - chstate:warn_uninit(accessing_node, action_key == "mutations") + table.insert(chstate.warnings, new_uninit_warning(accessing_node, action_key == "mutations")) end end end @@ -34,11 +44,11 @@ local function detect_uninit_access_in_line(chstate, line) end end --- Detects accesses that don't resolve to any values except initial empty one. -local function detect_uninit_access(chstate, line) - detect_uninit_access_in_line(chstate, line) +-- Adds warnings for accesses that don't resolve to any values except initial empty one. +local function detect_uninit_access(chstate) + detect_uninit_access_in_line(chstate, chstate.main_line) - for _, nested_line in ipairs(line.lines) do + for _, nested_line in ipairs(chstate.main_line.lines) do detect_uninit_access_in_line(chstate, nested_line) end end diff --git a/src/luacheck/detect_unreachable_code.lua b/src/luacheck/detect_unreachable_code.lua index d4dc45c7..718e09c2 100644 --- a/src/luacheck/detect_unreachable_code.lua +++ b/src/luacheck/detect_unreachable_code.lua @@ -1,5 +1,14 @@ local core_utils = require "luacheck.core_utils" +local function new_unreachable_code_warning(location, is_unrepeatable_loop, token) + return { + code = "51" .. (is_unrepeatable_loop and "2" or "1"), + line = location.line, + column = location.column, + end_column = location.column + #token - 1 + } +end + local function noop_callback() end local function detect_unreachable_code_in_line(chstate, line) @@ -14,7 +23,7 @@ local function detect_unreachable_code_in_line(chstate, line) for i, item in ipairs(line.items) do if not reachable_indexes[i] then if item.location then - chstate:warn_unreachable(item.location, item.loop_end, item.token) + table.insert(chstate.warnings, new_unreachable_code_warning(item.location, item.loop_end, item.token)) -- Mark all items reachable from the item just reported. core_utils.walk_line(line, reachable_indexes, i, noop_callback) end @@ -22,10 +31,11 @@ local function detect_unreachable_code_in_line(chstate, line) end end -local function detect_unreachable_code(chstate, line) - detect_unreachable_code_in_line(chstate, line) +-- Adds warnings for unreachable code. +local function detect_unreachable_code(chstate) + detect_unreachable_code_in_line(chstate, chstate.main_line) - for _, nested_line in ipairs(line.lines) do + for _, nested_line in ipairs(chstate.main_line.lines) do detect_unreachable_code_in_line(chstate, nested_line) end end diff --git a/src/luacheck/detect_unused_locals.lua b/src/luacheck/detect_unused_locals.lua index 531a7bd7..22c8eba3 100644 --- a/src/luacheck/detect_unused_locals.lua +++ b/src/luacheck/detect_unused_locals.lua @@ -1,6 +1,77 @@ local core_utils = require "luacheck.core_utils" local utils = require "luacheck.utils" +local function is_secondary(value) + return value.secondaries and value.secondaries.used +end + +local type_codes = { + var = "1", + func = "1", + arg = "2", + loop = "3", + loopi = "3" +} + +local function new_unused_var_warning(value, is_useless) + return { + code = "21" .. type_codes[value.var.type], + name = value.var.name, + line = value.location.line, + column = value.location.column, + end_column = value.location.column + (value.var.self and #":" or #value.var.name) - 1, + secondary = is_secondary(value) or nil, + func = (value.type == "func") or nil, + self = value.var.self, + useless = value.var.name == "_" and is_useless or nil + } +end + +local function new_unset_var_warning(var) + return { + code = "221", + name = var.name, + line = var.location.line, + column = var.location.column, + end_column = var.location.column + #var.name - 1 + } +end + +local function new_unaccessed_var_warning(var, was_mutated) + -- Mark as secondary if all assigned values are secondary. + -- It is guaranteed that there are at least two values. + local secondary = true + + for _, value in ipairs(var.values) do + if not value.empty and not is_secondary(value) then + secondary = nil + break + end + end + + return { + code = "2" .. (was_mutated and "4" or "3") .. type_codes[var.type], + name = var.name, + line = var.location.line, + column = var.location.column, + end_column = var.location.column + (var.self and #":" or #var.name) - 1, + secondary = secondary + } +end + +local function new_unused_value_warning(value, was_mutated, overwriting_node) + return { + code = "3" .. (was_mutated and "3" or "1") .. type_codes[value.type], + name = value.var.name, + overwritten_line = overwriting_node and overwriting_node.location.line, + overwritten_column = overwriting_node and overwriting_node.location.column, + line = value.location.line, + column = value.location.column, + end_column = value.location.column + (value.type == "arg" and value.var.self and #":" or #value.var.name) - 1, + secondary = is_secondary(value) or nil + } +end + local externally_accessible_tags = utils.array_to_set({"Id", "Index", "Call", "Invoke", "Op", "Paren", "Dots"}) local function externally_accessible(value) @@ -34,22 +105,22 @@ local function check_var(chstate, var) local value = var.values[2] or var.values[1] if not value.used then - chstate:warn_unused_variable(value) + table.insert(chstate.warnings, new_unused_var_warning(value)) end elseif #var.values == 1 then if not var.values[1].used then if var.values[1].mutated then if not externally_accessible(var.values[1]) then - chstate:warn_unaccessed(var, true) + table.insert(chstate.warnings, new_unaccessed_var_warning(var, true)) end else - chstate:warn_unused_variable(var.values[1], nil, nil, var.values[1].empty) + table.insert(chstate.warnings, new_unused_var_warning(var.values[1], var.values[1].empty)) end elseif var.values[1].empty then - chstate:warn_unset(var) + table.insert(chstate.warnings, new_unset_var_warning(var)) end elseif not var.accessed and not var.mutated then - chstate:warn_unaccessed(var) + table.insert(chstate.warnings, new_unaccessed_var_warning(var)) else local no_values_externally_accessible = true @@ -60,7 +131,7 @@ local function check_var(chstate, var) end if not var.accessed and no_values_externally_accessible then - chstate:warn_unaccessed(var, true) + table.insert(chstate.warnings, new_unaccessed_var_warning(var, true)) end for _, value in ipairs(var.values) do @@ -78,10 +149,10 @@ local function check_var(chstate, var) overwriting_node = get_overwriting_node_in_dup_assignment(value.item, value) end - chstate:warn_unused_value(value, false, overwriting_node) + table.insert(chstate.warnings, new_unused_value_warning(value, false, overwriting_node)) elseif not value.used and not externally_accessible(value) then if var.accessed or not no_values_externally_accessible then - chstate:warn_unused_value(value, true) + table.insert(chstate.warnings, new_unused_value_warning(value, true)) end end end @@ -104,10 +175,10 @@ end -- Detects unused local variables and their values as well as locals that -- are accessed but never set or set but never accessed. -local function detect_unused_locals(chstate, line) - detect_unused_locals_in_line(chstate, line) +local function detect_unused_locals(chstate) + detect_unused_locals_in_line(chstate, chstate.main_line) - for _, nested_line in ipairs(line.lines) do + for _, nested_line in ipairs(chstate.main_line.lines) do detect_unused_locals_in_line(chstate, nested_line) end end diff --git a/src/luacheck/detect_unused_rec_funcs.lua b/src/luacheck/detect_unused_rec_funcs.lua index 74343e09..e7c78fcf 100644 --- a/src/luacheck/detect_unused_rec_funcs.lua +++ b/src/luacheck/detect_unused_rec_funcs.lua @@ -1,5 +1,28 @@ local core_utils = require "luacheck.core_utils" +local function new_unused_rec_func_var_warning(value, is_self_recursive) + return { + code = "211", + name = value.var.name, + line = value.location.line, + column = value.location.column, + end_column = value.location.column + #value.var.name - 1, + func = true, + mutually_recursive = not is_self_recursive or nil, + recursive = is_self_recursive or nil + } +end + +local function new_unused_rec_func_value_warning(value) + return { + code = "311", + name = value.var.name, + line = value.location.line, + column = value.location.column, + end_column = value.location.column + #value.var.name - 1 + } +end + local function mark_reachable_lines(edges, marked, line) for connected_line in pairs(edges[line]) do if not marked[connected_line] then @@ -10,7 +33,7 @@ local function mark_reachable_lines(edges, marked, line) end -- Detects and reports unused recursive and mutually recursive functions. -local function detect_unused_rec_funcs(chstate, line) +local function detect_unused_rec_funcs(chstate) -- Build a graph of usage relations of all closures. -- Closure A is used by closure B iff either B is parent -- of A and A is not assigned to a local/upvalue, or @@ -18,6 +41,8 @@ local function detect_unused_rec_funcs(chstate, line) -- Closures not reachable from root closure are unused, -- report corresponding values/variables if not done already. + local line = chstate.main_line + -- Initialize edges maps. local forward_edges = {[line] = {}} local backward_edges = {[line] = {}} @@ -77,13 +102,13 @@ local function detect_unused_rec_funcs(chstate, line) value = mut_rec_line.node.value if value then - -- Report this closure as simply recursive or mutually recursive. - local simply_recursive = forward_edges[mut_rec_line][mut_rec_line] + -- Report this closure as self recursive or mutually recursive. + local is_self_recursive = forward_edges[mut_rec_line][mut_rec_line] if core_utils.is_function_var(value.var) then - chstate:warn_unused_variable(value, true, simply_recursive) + table.insert(chstate.warnings, new_unused_rec_func_var_warning(value, is_self_recursive)) else - chstate:warn_unused_value(value, true, simply_recursive) + table.insert(chstate.warnings, new_unused_rec_func_value_warning(value)) end end end diff --git a/src/luacheck/inline_options.lua b/src/luacheck/inline_options.lua index 950ddb27..6b2b907d 100644 --- a/src/luacheck/inline_options.lua +++ b/src/luacheck/inline_options.lua @@ -300,10 +300,10 @@ end -- and option events carry inline option tables themselves. -- Inline option errors have codes `02[123]`, issued for invalid option syntax, -- unpaired push directives and unpaired pop directives. -function inline_options.get_events(ast, comments, code_lines, warnings) - local events = utils.update({}, warnings) - add_closure_boundaries(ast, events) - local per_line_opts = add_inline_options(events, comments, code_lines) +function inline_options.get_events(chstate) + local events = utils.update({}, chstate.warnings) + add_closure_boundaries(chstate.ast, events) + local per_line_opts = add_inline_options(events, chstate.comments, chstate.code_lines) core_utils.sort_by_location(events) mark_unpaired_boundaries(events) events = filter_useless_boundaries(events) diff --git a/src/luacheck/linearize.lua b/src/luacheck/linearize.lua index 18ff5733..232c7e8a 100644 --- a/src/luacheck/linearize.lua +++ b/src/luacheck/linearize.lua @@ -1,9 +1,63 @@ local parser = require "luacheck.parser" local utils = require "luacheck.utils" +local function new_unused_field_value_warning(node, overwriting_node) + return { + code = "314", + field = node.field, + index = node.is_index, + overwritten_line = overwriting_node.location.line, + overwritten_column = overwriting_node.location.column, + line = node.location.line, + column = node.location.column, + end_column = node.location.column + #node.first_token - 1 + } +end + +local type_codes = { + var = "1", + func = "1", + arg = "2", + loop = "3", + loopi = "3" +} + +local function new_redefined_warning(var, prev_var, is_same_scope) + return { + code = "4" .. (is_same_scope and "1" or (var.line == prev_var.line and "2" or "3")) .. type_codes[prev_var.type], + name = var.name, + line = var.location.line, + column = var.location.column, + end_column = var.location.column + (var.self and #":" or #var.name) - 1, + self = var.self and prev_var.self, + prev_line = prev_var.location.line, + prev_column = prev_var.location.column + } +end + +local function new_unused_label_warning(label) + return { + code = "521", + label = label.name, + line = label.location.line, + column = label.location.column, + end_column = label.end_column + } +end + +local function new_unbalanced_assignment_warning(node) + local has_shorter_lhs = #node[1] < #node[2] + + return { + code = "53" .. (has_shorter_lhs and "1" or "2"), + line = node.equals_location.line, + column = node.equals_location.column, + end_column = node.equals_location.column + } +end + local pseudo_labels = utils.array_to_set({"do", "else", "break", "end", "return"}) --- Who needs classes anyway. local function new_line(node, parent, value) return { accessed_upvalues = {}, -- Maps variables to arrays of accessing items. @@ -172,7 +226,7 @@ function LinState:leave_scope() for name, label in pairs(left_scope.labels) do if not label.used and not pseudo_labels[name] then - self.chstate:warn_unused_label(label) + table.insert(self.chstate.warnings, new_unused_label_warning(label)) end end @@ -186,10 +240,13 @@ function LinState:register_var(node, type_) local prev_var = self:resolve_var(var.name) if prev_var then - local same_scope = self.scopes.top.vars[var.name] - self.chstate:warn_redefined(var, prev_var, same_scope) + local is_same_scope = self.scopes.top.vars[var.name] + + if var.name ~= "..." then + table.insert(self.chstate.warnings, new_redefined_warning(var, prev_var, is_same_scope)) + end - if same_scope then + if is_same_scope then prev_var.scope_end = self.lines.top.items.size end end @@ -233,20 +290,30 @@ function LinState:register_label(name, location, end_column) self.scopes.top.labels[name] = new_label(self.lines.top, name, location, end_column) end --- `node` is assignment node (`Local or `Set). -function LinState:check_balance(node) - if node[2] then - if #node[1] < #node[2] then - self.chstate:warn_unbalanced(node.equals_location, true) - elseif (#node[1] > #node[2]) and node.tag ~= "Local" and not is_unpacking(node[2][#node[2]]) then - self.chstate:warn_unbalanced(node.equals_location) - end +function LinState:check_assignment_balance(node) + local lhs = node[1] + local rhs = node[2] + + if not rhs then + return + end + + if (#lhs < #rhs) or ((#lhs > #rhs) and node.tag == "Set" and not is_unpacking(rhs[#rhs])) then + table.insert(self.chstate.warnings, new_unbalanced_assignment_warning(node)) end end +-- Block is either a `Do` statement or a `Then` block within an `If` statement. function LinState:check_empty_block(block) if #block == 0 then - self.chstate:warn_empty_block(block.location, block.tag == "Do") + local is_do_block = block.tag == "Do" + + table.insert(self.chstate.warnings, { + code = "54" .. (is_do_block and "1" or "2"), + line = block.location.line, + column = block.location.column, + end_column = block.location.column + (is_do_block and #"do" or #"then") - 1 + }) end end @@ -425,7 +492,7 @@ LinState.emit_stmt_Call = LinState.emit_expr LinState.emit_stmt_Invoke = LinState.emit_expr function LinState:emit_stmt_Local(node) - self:check_balance(node) + self:check_assignment_balance(node) local item = new_local_item(node[1], node[2], node.location, node.first_token) self:emit(item) @@ -444,7 +511,7 @@ function LinState:emit_stmt_Localrec(node) end function LinState:emit_stmt_Set(node) - self:check_balance(node) + self:check_assignment_balance(node) local item = new_set_item(node[1], node[2], node.location, node.first_token) self:scan_exprs(item, node[2]) @@ -597,7 +664,7 @@ function LinState:scan_expr_Table(item, node) if field then if key_to_node[key] then - self.chstate:warn_unused_field_value(key_to_node[key], pair) + table.insert(self.chstate.warnings, new_unused_field_value_warning(key_to_node[key], pair)) end key_to_node[key] = pair @@ -694,14 +761,13 @@ function LinState:scan_expr_Function(item, node) end end --- Builds linear representation of AST and returns it. --- Emits warnings: global, redefined/shadowed, unused field, unused label, unbalanced assignment, empty block. -local function linearize(chstate, ast) +-- Builds linear representation of AST and assigns it as `chstate.main_line`. +-- Adds warnings: redefined/shadowed, unused field, unused label, unbalanced assignment, empty block. +local function linearize(chstate) local linstate = LinState(chstate) - local line = linstate:build_line({{{tag = "Dots", "..."}}, ast}) + chstate.main_line = linstate:build_line({{{tag = "Dots", "..."}}, chstate.ast}) assert(linstate.lines.size == 0) assert(linstate.scopes.size == 0) - return line end return linearize diff --git a/src/luacheck/name_functions.lua b/src/luacheck/name_functions.lua index 32223806..cbc50cdd 100644 --- a/src/luacheck/name_functions.lua +++ b/src/luacheck/name_functions.lua @@ -64,8 +64,8 @@ end -- * Function assigned to a field: "foo.bar.baz". -- Function can be in a table assigned to a variable or a field, e.g. `foo.bar = {baz = function() ... end}`. -- * Otherwise: `nil`. -local function name_functions(ast) - handle_nodes(ast) +local function name_functions(chstate) + handle_nodes(chstate.ast) end return name_functions diff --git a/src/luacheck/analyze.lua b/src/luacheck/resolve_locals.lua similarity index 98% rename from src/luacheck/analyze.lua rename to src/luacheck/resolve_locals.lua index ce3c3e56..836a9efb 100644 --- a/src/luacheck/analyze.lua +++ b/src/luacheck/resolve_locals.lua @@ -141,7 +141,6 @@ local function propagate_main_assignments(line) end end - -- Called when closure creation propagation reaches a line item. local function closure_creation_propagation_callback(line, _, item, propagated_line) if not item then @@ -209,12 +208,12 @@ end -- Finds reaching assignments for all local variable accesses. -local function analyze(line) - analyze_line(line) +local function resolve_locals(chstate) + analyze_line(chstate.main_line) - for _, nested_line in ipairs(line.lines) do + for _, nested_line in ipairs(chstate.main_line.lines) do analyze_line(nested_line) end end -return analyze +return resolve_locals