Nuclear foot-gun waiting to happen in pl.pretty.dump()

[alerque]

The pl.pretty module has a dump() function normally accepts two arguments, but it . This is quite useful most of the time. However it also comes with a built in nuclear foot-gun. Sometimes in the process of debugging one wants to dump the output of a function. If that function happens to return more than one value, the dump function will behave rather unexpectedly. The choice of having a filename as an optional second argument can set off some surprisingly bad results.

Take for example the Lua require() function. It can return more than one value and the second value will (not always but frequently) be the filename that the module resolved to:

(The absence of a second result in this case signals that this call did not have to load the module.) […] Besides that value, require also returns as a second result the loader data returned by the searcher, which indicates how require found the module.

Now consider trying to debug a C module you're working on:

local D = require("pl.pretty").dump

-- This is safe-ish
local mycmodule = require("mycmodule")
D(mycmodule)

Everything is still fine. But what if you tried to take the obvious shortcut just for a quick debug session and put the function call directly in the dump function? Well then it goes sideways:

$ lua -e 'require("pl.pretty").dump(require("mycmodule"))'
[1]    1785514 bus error (core dumped)  lua -e 'require("pl.pretty").dump(require("mycmodule"))'

That wasn't the expected answer. Core dumped? What even happened? Lets just try it again, maybe I typed it wrong:

$ lua -e 'require("pl.pretty").dump(require("mycmodule"))'
lua: error loading module 'mycmodule' from file '/home/caleb/projects/mycmodule/lua_modules/lib/lua/5.4/mycmodule.so':
        /home/caleb/projects/mycmodule/lua_modules/lib/lua/5.4/mycmodule.so: invalid ELF header
stack traceback:
        [C]: in ?
        [C]: in function 'require'
        (command line):1: in main chunk
        [C]: in ?

Huston we have a problem! Running the same command twice gave us a different error. But the real issues is we no longer have a C module at all. The file ./lua_modules/lib/lua/5.4/mycmodule.so is now a text file with the output of dump().

I'm not sure how to address this. Perhaps detecting and warning on possibly unintended inputs is possible. Perhaps changing the API in the next major version so it doesn't contain a foot-gun in the first place would be in order. Perhaps documenting this with a warning is all we can do.

[catwell]

In this specific case I would go with just removing the support for the filename in the next major version.

I mean this literally does:

pl.utils.writefile(filename, pl.pretty.write(t))

If you want to write to a file you can just explicitly do that.

OTOH you should probably not do that (note: I fixed the require - "pl.pretty" not "pl.require"):

local D = require("pl.pretty").dump

you should do that instead (which implicitly calls debug which does not have that problem):

local D = require("pl.pretty")

[Tieske]

This is a nasty coincidence of having an optional 2nd parameter, combined with the multi-return values in Lua.

Since this is mostly a debug function, I think we could append a default extension to the filename, eg. .dump or .debug to prevent any files being overwritten (not 100% free of collisions, but probably good enough). Combined with some solid warnings in the docs.