Skip to content

Commit

Permalink
Merge pull request #1208 from github/aeisenberg/better-error-message
Browse files Browse the repository at this point in the history
More readable error message for invalid `queries` block and invalid `query-filters` blocl
  • Loading branch information
aeisenberg committed Aug 23, 2022
2 parents 3e49948 + e379edd commit 1cd5043
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 16 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -2,7 +2,7 @@

## [UNRELEASED]

No user facing changes.
- Improve error messages when the code scanning configuration file includes an invalid `queries` block or an invalid `query-filters` block. [#1208](https://github.com/github/codeql-action/pull/1208)

## 2.1.20 - 22 Aug 2022

Expand Down
3 changes: 3 additions & 0 deletions lib/analyze.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/analyze.js.map

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions lib/analyze.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/analyze.test.js.map

Large diffs are not rendered by default.

11 changes: 7 additions & 4 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.

2 changes: 1 addition & 1 deletion 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.

10 changes: 10 additions & 0 deletions src/analyze.test.ts
Expand Up @@ -313,6 +313,16 @@ test("validateQueryFilters", (t) => {
},
{ message: /Only "include" or "exclude" filters are allowed/ }
);

t.throws(
() => {
return validateQueryFilters({ exclude: "foo" } as any);
},
{
message:
/Query filters must be an array of "include" or "exclude" entries/,
}
);
});

const convertPackToQuerySuiteEntryMacro = test.macro({
Expand Down
6 changes: 6 additions & 0 deletions src/analyze.ts
Expand Up @@ -593,6 +593,12 @@ export function validateQueryFilters(queryFilters?: configUtils.QueryFilter[]) {
return [];
}

if (!Array.isArray(queryFilters)) {
throw new Error(
`Query filters must be an array of "include" or "exclude" entries. Found ${typeof queryFilters}`
);
}

const errors: string[] = [];
for (const qf of queryFilters) {
const keys = Object.keys(qf);
Expand Down
2 changes: 1 addition & 1 deletion src/config-utils.test.ts
Expand Up @@ -1328,7 +1328,7 @@ doInvalidInputTest(
queries:
- uses:
- hello: world`,
configUtils.getQueryUsesInvalid
configUtils.getQueriesMissingUses
);

function doInvalidQueryUsesTest(
Expand Down
15 changes: 10 additions & 5 deletions src/config-utils.ts
Expand Up @@ -683,6 +683,14 @@ export function getQueriesInvalid(configFile: string): string {
);
}

export function getQueriesMissingUses(configFile: string): string {
return getConfigFilePropertyError(
configFile,
QUERIES_PROPERTY,
"must be an array, with each entry having a 'uses' property"
);
}

export function getQueryUsesInvalid(
configFile: string | undefined,
queryUses?: string
Expand Down Expand Up @@ -1181,11 +1189,8 @@ async function loadConfig(
throw new Error(getQueriesInvalid(configFile));
}
for (const query of queriesArr) {
if (
!(QUERIES_USES_PROPERTY in query) ||
typeof query[QUERIES_USES_PROPERTY] !== "string"
) {
throw new Error(getQueryUsesInvalid(configFile));
if (typeof query[QUERIES_USES_PROPERTY] !== "string") {
throw new Error(getQueriesMissingUses(configFile));
}
await parseQueryUses(
languages,
Expand Down

0 comments on commit 1cd5043

Please sign in to comment.