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 4e14024
Show file tree
Hide file tree
Showing 6 changed files with 89 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
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/d@1.2.3"],
[Language.java]: ["d/e", "f/g@1.2.3"],
},
[Language.cpp, Language.csharp],
{
[Language.cpp]: ["a/b", "c/d@1.2.3"],
}
);

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
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

0 comments on commit 4e14024

Please sign in to comment.