feat: support SARIF output formatter #120
Conversation
There was a problem hiding this comment.
Adding the SARIF output format sounds great. I'm sure there is legitimate uses for that ... I'm not convinced showing up in the "security" tab is what we want to have happen though. Most of the code lints luacheck produces are not likely to be security related.
Also a bit of a concern about deterministic testing...see code comments.
| it("has built-in Sarif formatter", function() | ||
| -- luacheck: ignore 631 | ||
| local expect = [[ | ||
| {"runs":[{"tool":{"driver":{"version":"1.0.0","name":"luacheck","informationUri":"https://github.com/lunarmodules/luacheck"}},"results":[{"locations":[{"physicalLocation":{"artifactLocation":{"uri":"bad_file"}}}],"ruleId":"I/O error","level":"error","message":{"text":"luacheck : fatal error F1: couldn't check bad_file: couldn't read: No such file or directory"}},{"locations":[{"physicalLocation":{"region":{"startLine":3,"startColumn":16},"artifactLocation":{"uri":"spec/samples/bad_code.lua"}}}],"ruleId":"W211","level":"warning","message":{"text":"spec/samples/bad_code.lua:3:16: unused function 'helper'"}},{"locations":[{"physicalLocation":{"region":{"startLine":3,"startColumn":23},"artifactLocation":{"uri":"spec/samples/bad_code.lua"}}}],"ruleId":"W212","level":"warning","message":{"text":"spec/samples/bad_code.lua:3:23: unused variable length argument"}},{"locations":[{"physicalLocation":{"region":{"startLine":7,"startColumn":10},"artifactLocation":{"uri":"spec/samples/bad_code.lua"}}}],"ruleId":"W111","level":"warning","message":{"text":"spec/samples/bad_code.lua:7:10: setting non-standard global variable 'embrace'"}},{"locations":[{"physicalLocation":{"region":{"startLine":8,"startColumn":10},"artifactLocation":{"uri":"spec/samples/bad_code.lua"}}}],"ruleId":"W412","level":"warning","message":{"text":"spec/samples/bad_code.lua:8:10: variable 'opt' was previously defined as an argument on line 7"}},{"locations":[{"physicalLocation":{"region":{"startLine":9,"startColumn":11},"artifactLocation":{"uri":"spec/samples/bad_code.lua"}}}],"ruleId":"W113","level":"warning","message":{"text":"spec/samples/bad_code.lua:9:11: accessing undefined variable 'hepler'"}},{"locations":[{"physicalLocation":{"region":{"startLine":1,"startColumn":6},"artifactLocation":{"uri":"spec/samples/python_code.lua"}}}],"ruleId":"E011","level":"error","message":{"text":"spec/samples/python_code.lua:1:6: expected '=' near '__future__'"}}]}],"version":"2.1.0","$schema":"https://json.schemastore.org/sarif-2.1.0.json"} |
There was a problem hiding this comment.
One nuance about Lua that makes it tricky to test reliably is that table key order is not deterministic. Obviously this isn't a Lua table it is a serialized JSON object, but what is to prevent a similar problem here? Both this expectation and the actual value are deserialized into Lua and then compared. How is that going to work out with the Lua table not having deterministic key ordering?
There was a problem hiding this comment.
I tried to compare the json strings directly. And although the content is the same, the key order of the json is different:
That's why I'm deserializing JSON to Lua Table for comparison.
The internal implementation of assert.same ignores the order of the lua table keys, ref: https://github.com/lunarmodules/luassert/blob/8b5fd81d942532877091b68f1f3bd0f4e78fba83/src/util.lua#L12
|
SARIF is a |
SARIF can be used by GitHub Code Scanning feature, which ill produce reports avaliable from the GitHub Security tab. I haven built a sample repository for the PR, you can use this sample repository to see the general effect.
In Security Tab: https://github.com/yeshan333/luacheck_sarif_report_demo/security/code-scanning
In Pull Request: yeshan333/luacheck_sarif_report_demo#1