Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow scans with packs for languages not being scanned #1116

Merged
merged 5 commits into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## [UNRELEASED]

No user facing changes.
- It is no longer an error to include an unscanned language in the `packs` block in the codeql configuration file. [#1116](https://github.com/github/codeql-action/pull/1116)
aeisenberg marked this conversation as resolved.
Show resolved Hide resolved

## 2.1.13 - 21 Jun 2022

Expand Down
23 changes: 15 additions & 8 deletions lib/config-utils.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/config-utils.js.map

Large diffs are not rendered by default.

20 changes: 15 additions & 5 deletions lib/config-utils.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/config-utils.test.js.map

Large diffs are not rendered by default.

49 changes: 37 additions & 12 deletions src/config-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { getCachedCodeQL, setCodeQL } from "./codeql";
import * as configUtils from "./config-utils";
import { createFeatureFlags, FeatureFlag } from "./feature-flags";
import { Language } from "./languages";
import { getRunnerLogger } from "./logging";
import { getRunnerLogger, Logger } from "./logging";
import { setupTests } from "./testing-utils";
import * as util from "./util";

Expand Down Expand Up @@ -1424,7 +1424,12 @@ const parsePacksMacro = test.macro({
expected: Partial<Record<Language, string[]>>
) =>
t.deepEqual(
configUtils.parsePacksFromConfig(packsByLanguage, languages, "/a/b"),
configUtils.parsePacksFromConfig(
packsByLanguage,
languages,
"/a/b",
mockLogger
),
expected
),

Expand All @@ -1446,7 +1451,8 @@ const parsePacksErrorMacro = test.macro({
configUtils.parsePacksFromConfig(
packsByLanguage as string[] | Record<string, string[]>,
languages,
"/a/b"
"/a/b",
{} as Logger
),
{
message: expected,
Expand Down Expand Up @@ -1499,6 +1505,19 @@ test(
}
);

test(
"two packs with unused language in config",
parsePacksMacro,
{
[Language.cpp]: ["a/b", "c/[email protected]"],
[Language.java]: ["d/e", "f/[email protected]"],
},
[Language.cpp, Language.csharp],
{
[Language.cpp]: ["a/b", "c/[email protected]"],
}
);

test(
"packs with other valid names",
parsePacksMacro,
Expand Down Expand Up @@ -1544,13 +1563,6 @@ test(
[Language.java, Language.python],
/The configuration file "\/a\/b" is invalid: property "packs" must split packages by language/
);
test(
"invalid language",
parsePacksErrorMacro,
{ [Language.java]: ["c/d"] },
[Language.cpp],
/The configuration file "\/a\/b" is invalid: property "packs" has "java", but it is not one of the languages to analyze/
);
test(
"not an array",
parsePacksErrorMacro,
Expand Down Expand Up @@ -1583,13 +1595,25 @@ function parseInputAndConfigMacro(
expected
) {
t.deepEqual(
configUtils.parsePacks(packsFromConfig, packsFromInput, languages, "/a/b"),
configUtils.parsePacks(
packsFromConfig,
packsFromInput,
languages,
"/a/b",
mockLogger
),
expected
);
}
parseInputAndConfigMacro.title = (providedTitle: string) =>
`Parse Packs input and config: ${providedTitle}`;

const mockLogger = {
info: (message: string) => {
console.log(message);
},
} as Logger;

function parseInputAndConfigErrorMacro(
t: ExecutionContext<unknown>,
packsFromConfig: string[] | Record<string, string[]>,
Expand All @@ -1603,7 +1627,8 @@ function parseInputAndConfigErrorMacro(
packsFromConfig,
packsFromInput,
languages,
"/a/b"
"/a/b",
mockLogger
);
},
{
Expand Down
30 changes: 20 additions & 10 deletions src/config-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -629,14 +629,11 @@ export function getPathsInvalid(configFile: string): string {
);
}

export function getPacksRequireLanguage(
lang: string,
configFile: string
): string {
function getPacksRequireLanguage(lang: string, configFile: string): string {
return getConfigFilePropertyError(
configFile,
PACKS_PROPERTY,
`has "${lang}", but it is not one of the languages to analyze`
`has "${lang}", but it is not a valid language.`
);
}

Expand Down Expand Up @@ -1026,7 +1023,8 @@ async function loadConfig(
parsedYAML[PACKS_PROPERTY] ?? {},
packsInput,
languages,
configFile
configFile,
logger
);

// If queries were provided using `with` in the action configuration,
Expand Down Expand Up @@ -1146,7 +1144,8 @@ const PACK_IDENTIFIER_PATTERN = (function () {
export function parsePacksFromConfig(
packsByLanguage: string[] | Record<string, string[]>,
languages: Language[],
configFile: string
configFile: string,
logger: Logger
): Packs {
const packs = {};

Expand All @@ -1168,7 +1167,16 @@ export function parsePacksFromConfig(
throw new Error(getPacksInvalid(configFile));
}
if (!languages.includes(lang as Language)) {
throw new Error(getPacksRequireLanguage(lang, configFile));
// This particular language is not being analyzed in this run.
adityasharad marked this conversation as resolved.
Show resolved Hide resolved
if (Language[lang as Language]) {
logger.info(
`Ignoring packs for ${lang} since this language is not being analyzed in this run.`
);
continue;
} else {
// This language is invalid, probably a misspelling
throw new Error(getPacksRequireLanguage(configFile, lang));
}
}
packs[lang] = [];
for (const packStr of packsArr) {
Expand Down Expand Up @@ -1296,13 +1304,15 @@ export function parsePacks(
rawPacksFromConfig: string[] | Record<string, string[]>,
rawPacksInput: string | undefined,
languages: Language[],
configFile: string
configFile: string,
logger: Logger
) {
const packsFromInput = parsePacksFromInput(rawPacksInput, languages);
const packsFomConfig = parsePacksFromConfig(
rawPacksFromConfig,
languages,
configFile
configFile,
logger
);

if (!packsFromInput) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ packs:
- dsp-testing/[email protected]
- dsp-testing/codeql-pack2
- dsp-testing/codeql-pack3:other-query.ql
ruby:
- dsp-testing/hucairz
- dsp-testing/[email protected]

paths-ignore:
- tests
Expand Down