Do not artificially restrict the set of supported languages #779
Conversation
esbena
added
enhancement
esbena
force-pushed
the
esbena/no-lang
branch
from
6d69008 to
3847055
Compare
|
This is generally a good idea, IMO, especially as we start building out more languages. |
| @@ -299,7 +299,7 @@ test("load non-empty input", async (t) => { | |||
|
|
|||
| // And the config we expect it to parse to | |||
| const expectedConfig: configUtils.Config = { | |||
| languages: [Language.javascript], | |||
| languages: [KnownLanguage.javascript], | |||
There was a problem hiding this comment.
I think we should have a new unit test here for a language is not a known language, but it is valid since it was returned by a mocked call to codeql resolve languages.
| @@ -1172,7 +1174,7 @@ function shouldCombinePacks(packsInput?: string): boolean { | |||
| function combinePacks(packs1: Packs, packs2: Packs): Packs { | |||
| const packs = {}; | |||
| for (const lang of Object.keys(packs1)) { | |||
| packs[lang] = packs1[lang].concat(packs2[lang] || []); | |||
| packs[lang] = packs1[lang]?.concat(packs2[lang] || []); | |||
There was a problem hiding this comment.
I think this is what we want to do in order to ensure we never set packs[lang] to undefined.
| packs[lang] = packs1[lang]?.concat(packs2[lang] || []); | |
| packs[lang] = (packs1[lang] || []).concat(packs2[lang] || []); |
| (process.env["CODEQL_EXTRACTOR_GO_BUILD_TRACING"] === "on" && | ||
| language === Language.go) | ||
| language === KnownLanguage.go) |
There was a problem hiding this comment.
So any non KnownLanguage is assumed to be a scanned language? Is that correct?
There was a problem hiding this comment.
Yes. The default could go either way though.
For full flexibility, the action could take a parameter that signals the desired mode.
|
Will the extra extractors need to be packaged with the distribution, or are you expecting extractors to be resolvable from user supplied qlpacks? |
|
If you modify the codeql search path appropriately, you can use your own extractor in the action. We do not document how though. (See https://github.com/github/codeql-ql/blob/main/.github/workflows/nightly-changes.yml#L82) |
Description
The hardcoded set of supported languages in
languages.tsmakes it hard to experiment with new languages as custom branch with a modifiedlanguages.tsis required for every new language.This PR makes it easier to experiment with new languages by making the
codeql-actionindifferent to the language choice, except for some additional support for known languages.Implementation
The old implementation throws an early error if the user-provided language is not in the hardcoded set. As an alternative, a
Languageis now just a lowercase string, and an early error is only produced if that language is not in the output ofcodeql resolve languages. The oldLanguageenum is now namedKnownLanguage, and is used in the places where additional support for a specific language is desired.This should be fully backwards compatible, as the implementation now should throw an early exception in fewer situations. This may be detrimental to how early and well users are informed of input mistakes. On the other hand, it is consistent with how the provided
codeqland environment behave.PMs may want to keep the tight coupling between the codeql-action and the GitHub-controlled CodeQL repositories, I do not.
Testing
I will use this branch for some internal experiments for a short while. Hopefully, the many changes wont result in too many conflicts when we are ready to merge.
Merge / deployment checklist