Skip to content

Commit 7ee374f

Browse files
woodruffwDarkaMaul
woodruffw
and
DarkaMaul
authored
KATs for GitHub Actions expressions (#1857)
* Initial introduction of KAT * Fix some errors in the test harness * Use sparse checkout instead * Fix test harness * Rename KAT tests, docs and typesafety Signed-off-by: William Woodruff <william@yossarian.net> * Test case() Signed-off-by: William Woodruff <william@yossarian.net> * Implement GitHub's special "uppercase" semantics (#1879) * Implement GitHub's special "uppercase" semantics Signed-off-by: William Woodruff <william@yossarian.net> * fmt Signed-off-by: William Woodruff <william@yossarian.net> * Record changes Signed-off-by: William Woodruff <william@yossarian.net> --------- Signed-off-by: William Woodruff <william@yossarian.net> * Fix arity check for join() (#1880) * Fix arity check for join() Signed-off-by: William Woodruff <william@yossarian.net> * Clippy Signed-off-by: William Woodruff <william@yossarian.net> --------- Signed-off-by: William Woodruff <william@yossarian.net> * Simplify KAT result comparison Signed-off-by: William Woodruff <william@yossarian.net> * Fix number formatting Signed-off-by: William Woodruff <william@yossarian.net> * Use expect() Signed-off-by: William Woodruff <william@yossarian.net> * Skip the last WONTFIX'd KATs Signed-off-by: William Woodruff <william@yossarian.net> --------- Signed-off-by: William Woodruff <william@yossarian.net> Co-authored-by: Alexis <alexis.challande@trailofbits.com>
1 parent 7f70d0b commit 7ee374f

37 files changed

Lines changed: 6227 additions & 14 deletions

‎.github/workflows/codegen.yml‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,3 +125,42 @@ jobs:
125125
Please review manually before merging.
126126
assignees: ${{ env.PR_ASSIGNEES }}
127127
reviewers: ${{ env.PR_ASSIGNEES }}
128+
129+
refresh-expression-tests:
130+
name: Refresh expression test data 🧪
131+
runs-on: ubuntu-latest
132+
if: ${{ github.repository_owner == 'zizmorcore' }}
133+
134+
permissions:
135+
contents: write
136+
pull-requests: write
137+
138+
steps:
139+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
140+
with:
141+
persist-credentials: false
142+
143+
- uses: astral-sh/setup-uv@eac588ad8def6316056a12d4907a9d4d84ff7a3b # v7.3.0
144+
145+
- name: try to refresh expression tests
146+
run: |
147+
make sync-expression-tests
148+
149+
- name: create PR
150+
uses: peter-evans/create-pull-request@c0f553fe549906ede9cf27b5156039d195d2ece0 # v8.1.0
151+
with:
152+
draft: true
153+
commit-message: "[BOT] update expression test data from GitHub"
154+
branch: refresh-expression-tests
155+
branch-suffix: timestamp
156+
title: "[BOT] update expression test data from GitHub"
157+
body: |
158+
:robot: :warning: :robot:
159+
160+
This is an automated pull request, updating the expression
161+
test data after a change to GitHub's languageservices
162+
testdata was detected.
163+
164+
Please review manually before merging.
165+
assignees: ${{ env.PR_ASSIGNEES }}
166+
reviewers: ${{ env.PR_ASSIGNEES }}

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Makefile‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ codeql-injection-sinks: crates/zizmor/data/codeql-injection-sinks.json
4242
crates/zizmor/data/codeql-injection-sinks.json: support/codeql-injection-sinks.py
4343
$< > $@
4444

45+
.PHONY: sync-expression-tests
46+
sync-expression-tests:
47+
support/sync-expression-tests.py
48+
4549
.PHONY: archived-repos
4650
archived-repos:
4751
support/archived-repos.py

‎crates/github-actions-expressions/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,4 @@ subfeature.workspace = true
2323

2424
[dev-dependencies]
2525
pretty_assertions.workspace = true
26+
serde.workspace = true

‎crates/github-actions-expressions/src/call.rs‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Representation of function calls in GitHub Actions expressions.
22
3-
use crate::{Evaluation, SpannedExpr};
3+
use crate::{Evaluation, EvaluationSema, SpannedExpr};
44

55
/// Errors that can occur during parsing of function calls.
66
#[derive(Debug, thiserror::Error)]
@@ -89,7 +89,10 @@ impl<'src> Call<'src> {
8989
Function::ToJSON | Function::FromJSON if args.len() != 1 => {
9090
return Err(Error::Arity(func, "exactly 1 argument"));
9191
}
92-
Function::Join | Function::HashFiles if args.is_empty() => {
92+
Function::Join if args.is_empty() || args.len() > 2 => {
93+
return Err(Error::Arity(func, "1 or 2 arguments"));
94+
}
95+
Function::HashFiles if args.is_empty() => {
9396
return Err(Error::Arity(func, "at least 1 argument"));
9497
}
9598
Function::Case => {
@@ -293,9 +296,9 @@ impl<'src> Call<'src> {
293296
| Evaluation::Null,
294297
) => {
295298
// Case-insensitive comparison
296-
let string_str = search_string.sema().to_string().to_uppercase();
297-
let prefix_str = search_value.sema().to_string().to_uppercase();
298-
Some(Evaluation::Boolean(string_str.starts_with(&prefix_str)))
299+
let haystack = EvaluationSema::upper_special(&search_string.sema().to_string());
300+
let needle = EvaluationSema::upper_special(&search_value.sema().to_string());
301+
Some(Evaluation::Boolean(haystack.starts_with(&needle)))
299302
}
300303
// If either argument is not primitive (array or dictionary), return false
301304
_ => Some(Evaluation::Boolean(false)),
@@ -326,9 +329,9 @@ impl<'src> Call<'src> {
326329
| Evaluation::Null,
327330
) => {
328331
// Case-insensitive comparison
329-
let string_str = search_string.sema().to_string().to_uppercase();
330-
let suffix_str = search_value.sema().to_string().to_uppercase();
331-
Some(Evaluation::Boolean(string_str.ends_with(&suffix_str)))
332+
let haystack = EvaluationSema::upper_special(&search_string.sema().to_string());
333+
let needle = EvaluationSema::upper_special(&search_value.sema().to_string());
334+
Some(Evaluation::Boolean(haystack.ends_with(&needle)))
332335
}
333336
// If either argument is not primitive (array or dictionary), return false
334337
_ => Some(Evaluation::Boolean(false)),

‎crates/github-actions-expressions/src/lib.rs‎

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -857,15 +857,37 @@ fn parse_number(s: &str) -> f64 {
857857
/// various evaluation semantics (comparison, stringification, etc.).
858858
pub struct EvaluationSema<'a>(&'a Evaluation);
859859

860+
impl EvaluationSema<'_> {
861+
/// Converts a string to its uppercase form using GitHub Actions'
862+
/// special rules.
863+
/// See `toUpperSpecial`:
864+
/// <https://github.com/actions/languageservices/blob/cc316ab/expressions/src/result.ts#L209>
865+
fn upper_special(value: &str) -> String {
866+
// Uppercase everything except the small dotless-ı (U+0131),
867+
// which GitHub Actions preserves as-is.
868+
let mut result = String::with_capacity(value.len());
869+
let mut parts = value.split('ı');
870+
if let Some(first) = parts.next() {
871+
result.extend(first.chars().flat_map(char::to_uppercase));
872+
}
873+
for part in parts {
874+
result.push('ı');
875+
result.extend(part.chars().flat_map(char::to_uppercase));
876+
}
877+
result
878+
}
879+
}
880+
860881
impl PartialEq for EvaluationSema<'_> {
861882
fn eq(&self, other: &Self) -> bool {
862883
match (self.0, other.0) {
863884
(Evaluation::Null, Evaluation::Null) => true,
864885
(Evaluation::Boolean(a), Evaluation::Boolean(b)) => a == b,
865886
(Evaluation::Number(a), Evaluation::Number(b)) => a == b,
866887
// GitHub Actions string comparisons are case-insensitive.
867-
(Evaluation::String(a), Evaluation::String(b)) => a.to_uppercase() == b.to_uppercase(),
868-
888+
(Evaluation::String(a), Evaluation::String(b)) => {
889+
Self::upper_special(a) == Self::upper_special(b)
890+
}
869891
// Coercion rules: all others convert to number and compare.
870892
(a, b) => a.as_number() == b.as_number(),
871893
}
@@ -879,7 +901,8 @@ impl PartialOrd for EvaluationSema<'_> {
879901
(Evaluation::Boolean(a), Evaluation::Boolean(b)) => a.partial_cmp(b),
880902
(Evaluation::Number(a), Evaluation::Number(b)) => a.partial_cmp(b),
881903
(Evaluation::String(a), Evaluation::String(b)) => {
882-
a.to_uppercase().partial_cmp(&b.to_uppercase())
904+
// GitHub Actions string comparisons are case-insensitive.
905+
Self::upper_special(a).partial_cmp(&Self::upper_special(b))
883906
}
884907
// Coercion rules: all others convert to number and compare.
885908
(a, b) => a.as_number().partial_cmp(&b.as_number()),
@@ -893,10 +916,22 @@ impl std::fmt::Display for EvaluationSema<'_> {
893916
Evaluation::String(s) => write!(f, "{}", s),
894917
Evaluation::Number(n) => {
895918
// Format numbers like GitHub Actions does
896-
if n.fract() == 0.0 {
897-
write!(f, "{}", *n as i64)
919+
if n == &f64::INFINITY {
920+
write!(f, "Infinity")
921+
} else if n == &f64::NEG_INFINITY {
922+
write!(f, "-Infinity")
898923
} else {
899-
write!(f, "{}", n)
924+
// Format with 15 decimal places, parse back to f64 to
925+
// clean up trailing noise, then format normally.
926+
// See: https://github.com/actions/languageservices/blob/cc316ab/expressions/src/data/number.ts#L10
927+
let rounded: f64 = format!("{:.15}", n)
928+
.parse()
929+
.expect("impossible f64 round-trip error");
930+
if rounded.fract() == 0.0 {
931+
write!(f, "{}", rounded as i64)
932+
} else {
933+
write!(f, "{}", rounded)
934+
}
900935
}
901936
}
902937
Evaluation::Boolean(b) => write!(f, "{}", b),
@@ -2138,4 +2173,29 @@ mod tests {
21382173

21392174
Ok(())
21402175
}
2176+
2177+
#[test]
2178+
fn test_upper_special() {
2179+
use super::EvaluationSema;
2180+
2181+
let cases = &[
2182+
("", ""),
2183+
("abc", "ABC"),
2184+
("ıabc", "ıABC"),
2185+
("ııabc", "ııABC"),
2186+
("abcı", "ABCı"),
2187+
("abcıı", "ABCıı"),
2188+
("abcıdef", "ABCıDEF"),
2189+
("abcııdef", "ABCııDEF"),
2190+
("abcıdefıghi", "ABCıDEFıGHI"),
2191+
];
2192+
2193+
for (input, want) in cases {
2194+
assert_eq!(
2195+
EvaluationSema::upper_special(input),
2196+
*want,
2197+
"input: {input}"
2198+
);
2199+
}
2200+
}
21412201
}

0 commit comments

Comments
 (0)