Skip to content

Commit

Permalink
Allow scans with packs for languages not being scanned
Browse files Browse the repository at this point in the history
Previously, we were being too strict about checking that a pack's
language was being scanned. It was a failure if a pack language
was specified for a language not being scanned.
  • Loading branch information
aeisenberg committed Jun 22, 2022
1 parent 2e0c6ca commit 1653a84
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 37 deletions.
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.
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

0 comments on commit 1653a84

Please sign in to comment.