Skip to content

Commit

Permalink
Fix event sorting
Browse files Browse the repository at this point in the history
A warning and an inline option boundary event can share location
when a function declaration is unreachable code (a push event is generated
for each closure start). Previously event comparator assumed that only two
warnings or two inline option events can share location, resulting in an
error when this assumption is violated.

The fix is to compare events of different types by set priority before
falling back to code comparison for warning. Also remove a hack
that partially worked around this issue by adding `code` field
to some inline option events.

Ref #74.
  • Loading branch information
mpeterv committed Oct 25, 2016
1 parent 48fd788 commit 15890fa
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 10 deletions.
11 changes: 11 additions & 0 deletions spec/check_spec.lua
Expand Up @@ -617,6 +617,17 @@ end
]])
end)

it("detects unreachable functions", function()
assert.same({
{code = "231", name = "f", line = 1, column = 7, end_column = 7},
{code = "511", line = 3, column = 1, end_column = 8}
}, check[[
local f = nil
do return end
function f() end
]])
end)

it("detects unreachable code in nested function", function()
assert.same({
{code = "511", line = 4, column = 7, end_column = 12}
Expand Down
31 changes: 24 additions & 7 deletions src/luacheck/core_utils.lua
Expand Up @@ -58,16 +58,33 @@ function core_utils.is_definition(opts, warning)
return opts.allow_defined or (opts.allow_defined_top and warning.top)
end

local function location_comparator(event1, event2)
-- If two events share location, neither can be an invalid comment event.
-- However, they can be equal by identity due to the way table.sort is implemented.
return event1.line < event2.line or
event1.line == event2.line and (event1.column < event2.column or
event1.column == event2.column and event1.code and event1.code < event2.code)
local function event_priority(event)
-- Inline option boundaries have priority over inline option declarations
-- so that `-- luacheck: push ignore foo` is interpreted correctly (push first).
if event.push or event.pop then
return -2
elseif event.options then
return -1
else
return tonumber(event.code)
end
end

local function event_comparator(event1, event2)
if event1.line ~= event2.line then
return event1.line < event2.line
elseif event1.column ~= event2.column then
return event1.column < event2.column
else
return event_priority(event1) < event_priority(event2)
end
end

-- Sorts an array of warnings, inline options (tables with `options` field)
-- or inline option boundaries (tables with `push` or `pop` field) by location
-- information as provided in `line` and `column` fields.
function core_utils.sort_by_location(array)
table.sort(array, location_comparator)
table.sort(array, event_comparator)
end

return core_utils
6 changes: 3 additions & 3 deletions src/luacheck/inline_options.lua
Expand Up @@ -98,7 +98,7 @@ local function add_inline_option(events, per_line_opts, body, location, end_colu
end

if body == "push" or body == "pop" then
table.insert(events, {code = 1, [body] = true, line = location.line, column = location.column, end_column = end_column})
table.insert(events, {[body] = true, line = location.line, column = location.column, end_column = end_column})

if after_push then
body = after_push
Expand All @@ -120,7 +120,7 @@ local function add_inline_option(events, per_line_opts, body, location, end_colu

table.insert(per_line_opts[location.line], opts)
else
table.insert(events, {code = 2, options = opts, line = location.line, column = location.column, end_column = end_column})
table.insert(events, {options = opts, line = location.line, column = location.column, end_column = end_column})
end

return true
Expand Down Expand Up @@ -216,7 +216,7 @@ local function handle_events(events, per_line_opts)

-- Go through all events.
for _, event in ipairs(events) do
if type(event.code) == "string" then
if event.code then
-- It's a warning, put it into list of not handled warnings.
table.insert(unfiltered_warnings, event)
elseif event.options then
Expand Down

0 comments on commit 15890fa

Please sign in to comment.