Skip to content

Commit 6bc715a

Browse files
chore(typst): clean up dead code and one more global leak
- parse_multiple was declared without `local`, leaking as a global on every module load. Made it `local function` (forward use in translate_border still works since the local is declared above). - Removed `set_brand_mode = set_brand_mode,` from the export table: set_brand_mode is never defined in this module and has no callers anywhere, so the export silently resolved to nil — a phantom API surface. - Removed dead `local mult = 1` initial assignment (immediately overwritten in every branch); now just `local mult` then assigned. - Removed unused `local dash` in translate_border_style. - Removed a duplicate `local function quote(s)` definition (two identical copies of the same function body, the first one dead). - Removed a trailing-whitespace nit. Two regression tests added in tests/unit-lua/typst-css.test.lua: - TestModuleNoGlobalLeak.testParseMultipleNotGlobal — `_G.parse_multiple` must be nil after the module loads. - TestModuleExports.testNoPhantomExports — every key listed on the LHS of the module's `return { ... }` block must resolve to a non-nil value on the loaded module. (pairs() can't see this because Lua tables drop nil-valued keys, so the test reads the source file.)
1 parent 736b949 commit 6bc715a

2 files changed

Lines changed: 42 additions & 9 deletions

File tree

src/resources/filters/modules/typst_css.lua

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ local function output_color(color, opacity, warnings)
357357
end
358358
color = _quarto.format.typst.css.parse_color(cssColor)
359359
end
360-
local mult = 1
360+
local mult
361361
if opacity.unit == 'int' then
362362
mult = opacity.value / 255.9999
363363
elseif opacity.unit == 'percent' then
@@ -553,7 +553,7 @@ local function output_length(length, warnings)
553553
if not csf then
554554
output_warning(warnings, 'unit ' .. length.unit .. ' is not supported in ' .. length.csslen )
555555
return nil
556-
end
556+
end
557557
return csf(length.value, length.unit, length.csslen, warnings)
558558
end
559559

@@ -567,7 +567,7 @@ local border_styles = {
567567
'none', 'hidden', 'dotted', 'dashed', 'solid', 'double', 'groove', 'ridge', 'inset', 'outset', 'inherit', 'initial', 'revert', 'revert-layer', 'unset'
568568
}
569569

570-
function parse_multiple(s, limit, callback)
570+
local function parse_multiple(s, limit, callback)
571571
local start = 0
572572
local count = 0
573573
repeat
@@ -593,10 +593,6 @@ local function translate_border_width(v, warnings)
593593
return thickness == '0pt' and 'delete' or thickness
594594
end
595595

596-
local function quote(s)
597-
return '"' .. s .. '"'
598-
end
599-
600596
local same_weights = {
601597
'thin',
602598
'light',
@@ -660,7 +656,6 @@ end
660656

661657

662658
local function translate_border_style(v, _warnings)
663-
local dash
664659
if v == 'none' then
665660
return 'delete'
666661
elseif tcontains({'dotted', 'dashed'}, v) then
@@ -777,7 +772,6 @@ end
777772
return {
778773
quote = quote,
779774
dequote = dequote,
780-
set_brand_mode = set_brand_mode,
781775
parse_color = parse_color,
782776
parse_opacity = parse_opacity,
783777
output_color = output_color,

tests/unit-lua/typst-css.test.lua

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,4 +226,43 @@ function TestConsumeNoGlobalLeak:testConsumeStyleDoesNotLeak()
226226
lu.assertNil(rawget(_G, 'fend'), 'consume_style leaked global "fend"')
227227
end
228228

229+
-- The module's `function parse_multiple(...)` declaration is missing
230+
-- `local`, so loading the module installs `parse_multiple` as a global.
231+
TestModuleNoGlobalLeak = {}
232+
233+
function TestModuleNoGlobalLeak:testParseMultipleNotGlobal()
234+
lu.assertNil(rawget(_G, 'parse_multiple'),
235+
'loading typst_css leaked global "parse_multiple"')
236+
end
237+
238+
-- Every key listed on the LHS of the module's `return { ... }` table
239+
-- should resolve to a non-nil value on the loaded module. A `K = V,`
240+
-- entry where `V` is undefined silently turns the export into nil
241+
-- (Lua tables drop nil-valued keys), so `pairs(typst_css)` cannot see
242+
-- the bug — we have to read the source.
243+
TestModuleExports = {}
244+
245+
function TestModuleExports:testNoPhantomExports()
246+
local source_path = package.searchpath('typst_css', package.path)
247+
lu.assertNotNil(source_path, 'could not find typst_css.lua source')
248+
local f = assert(io.open(source_path, 'r'))
249+
local src = f:read('*all')
250+
f:close()
251+
252+
-- Anchor on `\nreturn {` and `\n}` so we pick the top-level export
253+
-- block, not an indented `return { ... }` inside a function body.
254+
local export_body = src:match('\nreturn%s*{(.-)\n}%s*$')
255+
lu.assertNotNil(export_body, 'could not locate top-level export table')
256+
257+
local nils = {}
258+
for k in export_body:gmatch('([%w_]+)%s*=') do
259+
if typst_css[k] == nil then
260+
table.insert(nils, k)
261+
end
262+
end
263+
lu.assertEquals(#nils, 0,
264+
'phantom exports (LHS in source but nil on module): ' ..
265+
table.concat(nils, ', '))
266+
end
267+
229268
os.exit(lu.LuaUnit.run())

0 commit comments

Comments
 (0)