Expand Azure Pipelines model foundation - #393
Open
Chanel (chanel-y) wants to merge 2 commits into
Open
Conversation
Add structural model coverage for Azure Pipelines documents and templates, including parameters, stages, jobs, checkout/template steps, script variants, and repository/pipeline resources. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8eab11c3-3153-4036-9847-28a2f615835f
Chanel (chanel-y)
force-pushed
the
iac-p0/ado-model-foundation
branch
from
August 26, 2026 17:03
c687a1d to
db8aea2
Compare
GitHub Actions workflows share `jobs:`/`steps:` keys with Azure DevOps pipelines, so the shape-based Document match misclassified them. Exclude documents that live under .github/workflows/ or declare a top-level `on:` trigger, and regenerate the AST test expectations. Adds a github-workflow.yml regression input. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mathias Vorreiter Pedersen (MathiasVP)
left a comment
Mathias Vorreiter Pedersen (MathiasVP)
left a comment
Collaborator
There was a problem hiding this comment.
Mostly stylistic comments. I think we should revisit all the predicate names / QLDoc for the "getters" implemented in this PR so that it conforms to the standard naming / phrasing conventions (i.e., the comment I put here: https://github.com/microsoft/codeql/pull/393/changes#r3926233771), but otherwise this all looks good!
Comment on lines
+44
to
+47
| /** | ||
| * Gets a top-level trigger-like entry. | ||
| */ | ||
| YamlValue getTrigger(string name) { result = this.lookup(name) } |
Collaborator
There was a problem hiding this comment.
Suggested change
| /** | |
| * Gets a top-level trigger-like entry. | |
| */ | |
| YamlValue getTrigger(string name) { result = this.lookup(name) } | |
| /** | |
| * Gets the top-level trigger-like entry named `name`, if any. | |
| */ | |
| YamlValue getTrigger(string name) { result = this.lookup(name) } |
Comment on lines
+54
to
+57
| /** | ||
| * Gets the pipeline parameters. | ||
| */ | ||
| Parameter getParameters() { result = this.lookup("parameters").getAChild() } |
Collaborator
There was a problem hiding this comment.
There's some QLDoc terminology that I'd really like us to switch to. A predicate can essentially fall into 4 categories:
- It always has 1 result
- It has at most 1 result (i.e., 0 or 1 result)
- It has 0 or more results
- It has 1 or more results
Each of these comes with recommended phrasing for the name of the predicate, and phrasing for the QLDoc:
- If the predicate always has 1 result it should be named
getX(for some appropriateX), and its QLDoc should be written asGets the X. For example:/** * Gets the parameter associated with this `Document`. */ Parameter getParameter() { result = .... }
- If the predicate has at most 1 result (i.e., 0 or 1) it should be named
getX, and its QLDoc should be written asGets the X, if any.. For example:/** * Gets the parameter associated with this `Document`, if any. */ Parameter getParameter() { result = .... }
- If the predicate has 0 or more results it should be named
getAX(orgetAnX), and its QLDoc should be written asGets an X, if any.. For example:/** * Gets a parameter associated with this `Document`, if any. */ Parameter getAParameter() { result = .... }
- If the predicate has 1 or more results it should be named
getAX(orgetAnX), and its QLDoc should be written asGets an X.. For example:/** * Gets a parameter associated with this `Document`. */ Parameter getAParameter() { result = .... }
The name getParameters doesn't fall into any of these 4 categories. So I don't know how many results I can expect from this query!
| class Stage extends YamlNode, YamlMapping { | ||
| Stage() { exists(Document document | document.lookup("stages").getAChildNode() = this) } | ||
|
|
||
| override string toString() { result = "Stage '" + this.getName() + "'" } |
Collaborator
There was a problem hiding this comment.
Remember this comment I made a while back in the context of TSQL support:
Based on that comment from old me, I would prefer we made this toString output just this.getName(). But if you think otherwise I'm happy to be corrected!
| exists(Stage stage | stage.lookup("jobs").getAChildNode() = this) | ||
| } | ||
|
|
||
| override string toString() { result = "Job '" + this.getName() + "'" } |
Collaborator
There was a problem hiding this comment.
Same here. I think this should just be result = this.getName().
Comment on lines
+283
to
+285
| exists(Document document | document.lookup("steps").getAChildNode() = this | result = document) | ||
| or | ||
| exists(Document document | this.getFile() = document.getFile() | result = document) |
Collaborator
There was a problem hiding this comment.
What a convoluted way to say this:
Suggested change
| exists(Document document | document.lookup("steps").getAChildNode() = this | result = document) | |
| or | |
| exists(Document document | this.getFile() = document.getFile() | result = document) | |
| result.lookup("steps").getAChildNode() = this | |
| or | |
| this.getFile() = result.getFile() |
Comment on lines
+395
to
+399
| exists(Document document | | ||
| document.lookup("resources").(YamlMapping).lookup("repositories").getAChildNode() = this | ||
| | | ||
| result = document | ||
| ) |
Collaborator
There was a problem hiding this comment.
Suggested change
| exists(Document document | | |
| document.lookup("resources").(YamlMapping).lookup("repositories").getAChildNode() = this | |
| | | |
| result = document | |
| ) | |
| result.lookup("resources").(YamlMapping).lookup("repositories").getAChildNode() = this |
Comment on lines
+434
to
+438
| exists(Document document | | ||
| document.lookup("resources").(YamlMapping).lookup("pipelines").getAChildNode() = this | ||
| | | ||
| result = document | ||
| ) |
Collaborator
There was a problem hiding this comment.
Suggested change
| exists(Document document | | |
| document.lookup("resources").(YamlMapping).lookup("pipelines").getAChildNode() = this | |
| | | |
| result = document | |
| ) | |
| result.lookup("resources").(YamlMapping).lookup("pipelines").getAChildNode() = this |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add structural model coverage for Azure Pipelines documents and templates, including parameters, stages, jobs, checkout/template steps, script variants, and repository/pipeline resources.