-
Notifications
You must be signed in to change notification settings - Fork 385
Resolve supported languages using CodeQL CLI #3083
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
Resolve supported languages using CodeQL CLI #3083
Conversation
Waiting on the next nightly to start integration testing this, but the code is ready for a first pass review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had an initial look over this and it looks good to me so far, thanks! There are just a couple of very minor points I thought of.
@@ -5,10 +5,11 @@ import type { VersionInfo } from "./codeql"; | |||
export enum ToolsFeature { | |||
AnalysisSummaryV2IsDefault = "analysisSummaryV2Default", | |||
DatabaseInterpretResultsSupportsSarifRunProperty = "databaseInterpretResultsSupportsSarifRunProperty", | |||
ForceOverwrite = "forceOverwrite", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: I assume you moved ForceOverwrite
so that the elements are in alphabetical order, but the new BuiltinExtractorsSpecifyDefaultQueries
entry is at the end. Should that come after AnalysisSummaryV2IsDefault
for consistency or was there another reason you moved ForceOverwrite
?
if (resolveSupportedLanguagesUsingCli) { | ||
logger.debug( | ||
`The CodeQL CLI supports the following languages: ${Object.keys(resolveResult.extractors).join(", ")}`, | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Since this is a .debug
call, we can probably skip the resolveSupportedLanguagesUsingCli
check since it wouldn't hurt having this in the debug output either way. Alternatively, I also wouldn't be opposed to making this an info
-level message if the resolveSupportedLanguagesUsingCli
check is kept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this info is only relevant if resolveSupportedLanguagesUsingCli
is true — otherwise it contains the whole list of extractors including HTML etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables resolving supported languages using the CodeQL CLI's codeql resolve languages --filter-to-languages-with-queries
command, behind a feature flag. This allows users to more easily start using new extractors with the CodeQL Action and simplifies adding new languages by dynamically determining which languages have default queries available.
Key changes:
- Adds a new feature flag
ResolveSupportedLanguagesUsingCli
to enable the CLI-based language resolution - Updates language resolution logic to optionally filter to languages with queries when the feature is enabled
- Modifies configuration utilities to pass feature enablement through the language detection pipeline
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/tools-features.ts |
Adds BuiltinExtractorsSpecifyDefaultQueries tools feature and reorganizes enum ordering |
src/feature-flags.ts |
Adds ResolveSupportedLanguagesUsingCli feature flag configuration with required tools feature |
src/config-utils.ts |
Updates language resolution to support CLI-based filtering and passes features through the pipeline |
src/config-utils.test.ts |
Adds comprehensive test coverage for both CLI and non-CLI language resolution modes |
src/codeql.ts |
Adds optional parameter to betterResolveLanguages for filtering to languages with queries |
pr-checks/checks/multi-language-autodetect.yml |
Enables the feature flag for testing |
.github/workflows/__multi-language-autodetect.yml |
Generated workflow file with feature flag enabled |
Comments suppressed due to low confidence (1)
src/config-utils.ts:1
- [nitpick] The function name
getDefaultConfig
doesn't clearly indicate what type of configuration it returns or that it's creating a new configuration object. Consider renaming tocreateDefaultConfig
orbuildDefaultConfig
to better reflect its purpose.
import * as fs from "fs";
BuiltinExtractorsSpecifyDefaultQueries = "builtinExtractorsSpecifyDefaultQueries", | ||
DatabaseInterpretResultsSupportsSarifRunProperty = "databaseInterpretResultsSupportsSarifRunProperty", | ||
IndirectTracingSupportsStaticBinaries = "indirectTracingSupportsStaticBinaries", | ||
SarifMergeRunsFromEqualCategory = "sarifMergeRunsFromEqualCategory", | ||
ForceOverwrite = "forceOverwrite", | ||
IndirectTracingSupportsStaticBinaries = "indirectTracingSupportsStaticBinaries", | ||
PythonDefaultIsToNotExtractStdlib = "pythonDefaultIsToNotExtractStdlib", | ||
SarifMergeRunsFromEqualCategory = "sarifMergeRunsFromEqualCategory", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The enum items are not in alphabetical order. Consider maintaining alphabetical order for better readability and maintainability.
Copilot uses AI. Check for mistakes.
This PR enables resolving supported languages using
codeql resolve languages --filter-to-languages-with-queries
, behind a feature flag. This allows users to more easily start using new extractors with the CodeQL Action and simplifies adding new languages.Risk assessment
For internal use only. Please select the risk level of this change:
Merge / deployment checklist