Allow the codeql-action to run packages - #545
aeisenberg merged 15 commits into
Conversation
Sorry, something went wrong.
This commit adds a `packs` option to the codeql-config.yml file. Users
can specify a list of ql packs to include in the analysis.
For a single language analysis, the packs property looks like this:
```yaml
packs:
- pack-scope/pack-name1@1.2.3
- pack-scope/pack-name2 # no explicit version means download the latest
```
For multi-language analysis, you must key the packs block by lanaguage:
```yaml
packs:
cpp:
- pack-scope/pack-name1@1.2.3
- pack-scope/pack-name2
java:
- pack-scope/pack-name3@1.2.3
- pack-scope/pack-name4
```
This implementation adds a new analysis run (alongside custom and
builtin runs). The unit tests indicate that the correct commands are
being run, but I have not actually tried this with a real CLI.
Also, convert `instanceof Array` to `Array.isArray` since that is
sightly better in some situations. See:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray#instanceof_vs_isarray
edoardopirovano
left a comment
There was a problem hiding this comment.
Mostly looks good, a few minor comments/questions
Sorry, something went wrong.
|
Is now a good point to create a feature branch? |
Sorry, something went wrong.
|
Sure. I'll create one now. |
Sorry, something went wrong.
During the analyze phase.
Use strings instead. They are easier to serialize and deserialize.
| languages: Language[], | ||
| configFile: string | ||
| ) { | ||
| const packs = {} as Packs; |
There was a problem hiding this comment.
Should Packs be updated such that {} would be valid? This sort of pattern appears in a few other places in the codebase. It might be that empty objects are intended to be valid for various configuration types (in which case it would make sense to update those types to allow for such shapes to be valid).
Sorry, something went wrong.
There was a problem hiding this comment.
I had the type definition wrong. Originally, it was:
export type Packs = Record<Partial<Language>, PackWithVersion[]>;But it should have been:
export type Packs = Partial<Record<Language, PackWithVersion[]>>;
Sorry, something went wrong.
|
|
||
| // Exported for testing | ||
| export function parsePacks( | ||
| packsByLanguage: string[] | Record<string, string[]> | undefined, |
There was a problem hiding this comment.
What is the use case for packsByLanguage to be undefined?
Sorry, something went wrong.
There was a problem hiding this comment.
This parameter is coming directly from the parsedYAML[PACKS_PROPERTY] property, parsed from the file. This field is optional and hence possibly undefined.
Sorry, something went wrong.
There was a problem hiding this comment.
Oh, there's a conditional block right above that call site that it should be brought up inside.
codeql-action/src/config-utils.ts
Line 1002 in d42f654
Sorry, something went wrong.
1. Better malformed data guard for PackDownloadOutput 2. Fix Packs type 3. Remove TODO in init-action
This commit adds a packs option to the codeql-config.yml file. Users
can specify a list of ql packs to include in the analysis.
For a single language analysis, the packs property looks like this:
For multi-language analysis, you must key the packs block by lanaguage:
This implementation adds a new analysis run (alongside custom and
builtin runs). The unit tests indicate that the correct commands are
being run, but I have not actually tried this with a real CLI.
Also, convert instanceof Array to Array.isArray since that is
sightly better in some situations. See:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray#instanceof_vs_isarray
Merge / deployment checklist