Skip to content

Commit 853ba06

Browse files
committed
Merge branch 'feature/CLDSRV-860/monitor-async-await-migration' into q/9.3
2 parents ef68e09 + 4682684 commit 853ba06

File tree

12 files changed

+614
-15
lines changed

12 files changed

+614
-15
lines changed

.github/codeql/async-migration.ql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Callback-style function (async migration)
3+
* @description These functions use callback parameters. They should be refactored to use async/await.
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @id js/callback-style-function
7+
* @tags maintainability
8+
* async-migration
9+
*/
10+
11+
import javascript
12+
13+
from Function f, Parameter p
14+
where
15+
p = f.getParameter(f.getNumParameter() - 1) and
16+
p.getName().regexpMatch("(?i)^(cb|callback|next|done)$") and
17+
not f.isAsync() and
18+
// Exclude test files and node_modules
19+
not f.getFile().getAbsolutePath().matches("%/tests/%") and
20+
not f.getFile().getAbsolutePath().matches("%/node_modules/%")
21+
select f, "This function uses a callback parameter ('" + p.getName() + "'). Refactor to async/await."

.github/codeql/async-suite.qls

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- description: Scality Cloudserver Async Migration Suite
2+
- queries: .
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name Promise .then() usage (async migration)
3+
* @description These calls use .then() instead of async/await. They should be refactored to use async/await.
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @id js/promise-then-usage
7+
* @tags maintainability
8+
* async-migration
9+
*/
10+
11+
import javascript
12+
13+
from MethodCallExpr m
14+
where
15+
m.getMethodName() = "then" and
16+
// Exclude test files and node_modules
17+
not m.getFile().getAbsolutePath().matches("%/tests/%") and
18+
not m.getFile().getAbsolutePath().matches("%/node_modules/%")
19+
select m, "This call uses .then(). Refactor to async/await."

.github/codeql/qlpack.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
name: scality/cloudserver-async-migration
2+
version: 0.0.1
3+
dependencies:
4+
codeql/javascript-all: "*"
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/**
2+
* Check that all new/modified functions in the current git diff use async/await.
3+
* Fails with exit code 1 if any additions introduce callback-style functions.
4+
*
5+
* Usage: node scripts/check-diff-async.mjs
6+
* In CI: runs against the current PR diff (files changed vs base branch)
7+
*/
8+
import { execFileSync } from 'node:child_process';
9+
import { Project, SyntaxKind } from 'ts-morph';
10+
11+
const CALLBACK_PARAM_PATTERN = /^(cb|callback|next|done)$/i;
12+
13+
function getChangedJsFiles() {
14+
const base = process.env.GITHUB_BASE_REF
15+
? `origin/${process.env.GITHUB_BASE_REF}`
16+
: 'HEAD';
17+
const output = execFileSync('git', [
18+
'diff',
19+
'--name-only',
20+
'--diff-filter=ACMR',
21+
base,
22+
'--',
23+
'**/*.js',
24+
], { encoding: 'utf8' }).trim();
25+
26+
return output ? output.split('\n').filter(f => f.endsWith('.js')) : [];
27+
}
28+
29+
/**
30+
* Get added line numbers for a file in the current diff.
31+
*/
32+
function getAddedLineNumbers(filePath) {
33+
const base = process.env.GITHUB_BASE_REF
34+
? `origin/${process.env.GITHUB_BASE_REF}`
35+
: 'HEAD';
36+
const diff = execFileSync('git', ['diff', base, '--', filePath], { encoding: 'utf8' });
37+
const addedLines = new Set();
38+
let currentLine = 0;
39+
40+
for (const line of diff.split('\n')) {
41+
const hunkMatch = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/);
42+
43+
if (hunkMatch) {
44+
currentLine = parseInt(hunkMatch[1], 10) - 1;
45+
continue;
46+
}
47+
48+
if (line.startsWith('+') && !line.startsWith('+++')) {
49+
currentLine++;
50+
addedLines.add(currentLine);
51+
} else if (!line.startsWith('-')) {
52+
currentLine++;
53+
}
54+
}
55+
56+
return addedLines;
57+
}
58+
59+
const changedFiles = getChangedJsFiles();
60+
if (changedFiles.length === 0) {
61+
console.log('No changed JS files to check.');
62+
process.exit(0);
63+
}
64+
65+
console.log(`Checking ${changedFiles.length} changed JS file(s) for async/await compliance...\n`);
66+
67+
const project = new Project({
68+
compilerOptions: { allowJs: true, noEmit: true },
69+
skipAddingFilesFromTsConfig: true,
70+
});
71+
72+
const filesToCheck = changedFiles.filter(f =>
73+
!f.startsWith('tests/') &&
74+
!f.startsWith('node_modules/') &&
75+
(
76+
f.startsWith('lib/') ||
77+
f.startsWith('bin/') ||
78+
!f.includes('/')
79+
)
80+
);
81+
if (filesToCheck.length === 0) {
82+
console.log('No source JS files in diff (tests and node_modules excluded).');
83+
process.exit(0);
84+
}
85+
86+
project.addSourceFilesAtPaths(filesToCheck);
87+
88+
const violations = [];
89+
90+
for (const sourceFile of project.getSourceFiles()) {
91+
const filePath = sourceFile.getFilePath().replace(process.cwd() + '/', '');
92+
const addedLines = getAddedLineNumbers(filePath);
93+
94+
if (addedLines.size === 0) continue;
95+
96+
const functions = [
97+
...sourceFile.getDescendantsOfKind(SyntaxKind.FunctionDeclaration),
98+
...sourceFile.getDescendantsOfKind(SyntaxKind.FunctionExpression),
99+
...sourceFile.getDescendantsOfKind(SyntaxKind.ArrowFunction),
100+
...sourceFile.getDescendantsOfKind(SyntaxKind.MethodDeclaration),
101+
];
102+
103+
for (const fn of functions) {
104+
if (fn.isAsync()) continue;
105+
106+
const startLine = fn.getStartLineNumber();
107+
if (!addedLines.has(startLine)) continue;
108+
109+
const params = fn.getParameters();
110+
const lastParam = params[params.length - 1];
111+
if (lastParam && CALLBACK_PARAM_PATTERN.test(lastParam.getName())) {
112+
violations.push({
113+
file: filePath,
114+
line: startLine,
115+
type: 'callback',
116+
detail: `function has callback parameter '${lastParam.getName()}'`,
117+
});
118+
}
119+
}
120+
}
121+
122+
if (violations.length === 0) {
123+
console.log('✓ All new code in the diff uses async/await.');
124+
process.exit(0);
125+
}
126+
127+
console.error(`✗ Found ${violations.length} async/await violation(s) in the diff:\n`);
128+
for (const v of violations) {
129+
console.error(` ${v.file}:${v.line} [${v.type}] ${v.detail}`);
130+
}
131+
console.error('\nNew code must use async/await instead of callbacks.');
132+
console.error('See the async/await migration guide in CONTRIBUTING.md for help.');
133+
process.exit(1);
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* Count async vs callback-style functions across the codebase using ts-morph.
3+
* Used in CI to track async/await migration progress over time.
4+
*
5+
* Usage: node scripts/count-async-functions.mjs
6+
*/
7+
import { readFileSync, appendFileSync, writeFileSync } from 'node:fs';
8+
import { Project, SyntaxKind } from 'ts-morph';
9+
10+
function getSourcePathsFromPackageJson() {
11+
const packageJsonPath = new URL('../../package.json', import.meta.url);
12+
const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8'));
13+
const paths = packageJson.countAsyncSourcePaths;
14+
15+
if (Array.isArray(paths) && paths.length > 0 && paths.every(p => typeof p === 'string')) {
16+
return paths;
17+
}
18+
19+
throw new Error('package.json must define a non-empty string array "countAsyncSourcePaths"');
20+
}
21+
22+
const project = new Project({
23+
compilerOptions: {
24+
allowJs: true,
25+
noEmit: true,
26+
},
27+
skipAddingFilesFromTsConfig: true,
28+
});
29+
30+
project.addSourceFilesAtPaths(getSourcePathsFromPackageJson());
31+
32+
let asyncFunctions = 0;
33+
let totalFunctions = 0;
34+
let callbackFunctions = 0;
35+
let thenChains = 0;
36+
37+
const CALLBACK_PARAM_PATTERN = /^(cb|callback|next|done)$/i;
38+
39+
for (const sourceFile of project.getSourceFiles()) {
40+
const functions = [
41+
...sourceFile.getDescendantsOfKind(SyntaxKind.FunctionDeclaration),
42+
...sourceFile.getDescendantsOfKind(SyntaxKind.FunctionExpression),
43+
...sourceFile.getDescendantsOfKind(SyntaxKind.ArrowFunction),
44+
...sourceFile.getDescendantsOfKind(SyntaxKind.MethodDeclaration),
45+
];
46+
47+
for (const fn of functions) {
48+
totalFunctions++;
49+
50+
if (fn.isAsync()) {
51+
asyncFunctions++;
52+
continue;
53+
}
54+
55+
const params = fn.getParameters();
56+
const lastParam = params[params.length - 1];
57+
if (lastParam && CALLBACK_PARAM_PATTERN.test(lastParam.getName())) {
58+
callbackFunctions++;
59+
}
60+
}
61+
62+
const propertyAccesses = sourceFile.getDescendantsOfKind(SyntaxKind.PropertyAccessExpression);
63+
for (const access of propertyAccesses) {
64+
if (access.getName() === 'then') {
65+
thenChains++;
66+
}
67+
}
68+
}
69+
70+
const asyncFunctionPercent = totalFunctions > 0
71+
? ((asyncFunctions / totalFunctions) * 100).toFixed(1)
72+
: '0.0';
73+
74+
const migrationPercent = (asyncFunctions + callbackFunctions) > 0
75+
? ((asyncFunctions / (asyncFunctions + callbackFunctions)) * 100).toFixed(1)
76+
: '0.0';
77+
78+
console.log('=== Async/Await Migration Progress ===');
79+
console.log(`Total functions: ${totalFunctions}`);
80+
console.log(`Async functions: ${asyncFunctions} (${asyncFunctionPercent}%)`);
81+
console.log(`Callback functions: ${callbackFunctions}`);
82+
console.log(`Remaining .then(): ${thenChains}`);
83+
console.log('');
84+
console.log(`Migration (trend): ${asyncFunctions}/${asyncFunctions + callbackFunctions} (${migrationPercent}%)`);
85+
86+
if (process.env.GITHUB_STEP_SUMMARY) {
87+
appendFileSync(process.env.GITHUB_STEP_SUMMARY, [
88+
'## Async/Await Migration Progress',
89+
'',
90+
`| Metric | Count |`,
91+
`|--------|-------|`,
92+
`| Total functions | ${totalFunctions} |`,
93+
`| Async functions | ${asyncFunctions} (${asyncFunctionPercent}%) |`,
94+
`| Callback-style functions | ${callbackFunctions} |`,
95+
`| Remaining \`.then()\` chains | ${thenChains} |`,
96+
`| Migration trend (async / (async + callback)) | ${asyncFunctions}/${asyncFunctions + callbackFunctions} (${migrationPercent}%) |`,
97+
'',
98+
].join('\n'));
99+
100+
// Output benchmark JSON for visualization
101+
const benchmarkData = [
102+
{
103+
name: 'Async Migration Progress',
104+
unit: '%',
105+
value: parseFloat(migrationPercent),
106+
},
107+
{
108+
name: 'Async Functions Percentage',
109+
unit: '%',
110+
value: parseFloat(asyncFunctionPercent),
111+
},
112+
{
113+
name: 'Total callback functions',
114+
unit: 'count',
115+
value: callbackFunctions,
116+
}
117+
];
118+
writeFileSync('async-migration-benchmark.json', JSON.stringify(benchmarkData, null, 2));
119+
}

.github/workflows/codeql.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ jobs:
2020
uses: github/codeql-action/init@v3
2121
with:
2222
languages: javascript, python, ruby
23+
queries: security-extended,./.github/codeql/async-suite.qls
2324

2425
- name: Build and analyze
2526
uses: github/codeql-action/analyze@v3

.github/workflows/tests.yaml

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ jobs:
8080
steps:
8181
- name: Checkout
8282
uses: actions/checkout@v4
83+
with:
84+
fetch-depth: 0
8385
- uses: actions/setup-node@v4
8486
with:
8587
node-version: '22'
@@ -95,14 +97,32 @@ jobs:
9597
cache: pip
9698
- name: Install python deps
9799
run: pip install flake8
98-
- name: Lint Javascript
99-
run: yarn run --silent lint -- --max-warnings 0
100+
- name: Lint Javascript (strict, excluding async migration rules)
101+
run: |
102+
yarn run --silent lint -- --max-warnings 0 --rule "promise/prefer-await-to-then: off" --rule "n/callback-return: off"
100103
- name: Lint Markdown
101104
run: yarn run --silent lint_md
102105
- name: Lint python
103106
run: flake8 $(git ls-files "*.py")
104107
- name: Lint Yaml
105108
run: yamllint -c yamllint.yml $(git ls-files "*.yml")
109+
- name: Check async/await compliance in diff
110+
run: yarn run check-diff-async
111+
112+
async-migration-report:
113+
runs-on: ubuntu-24.04
114+
if: startsWith(github.ref, 'refs/heads/development/')
115+
steps:
116+
- name: Checkout
117+
uses: actions/checkout@v4
118+
- uses: actions/setup-node@v4
119+
with:
120+
node-version: '22'
121+
cache: yarn
122+
- name: install dependencies
123+
run: yarn install --frozen-lockfile --network-concurrency 1
124+
- name: Count async/await migration progress
125+
run: yarn run count-async
106126

107127
unit-tests:
108128
runs-on: ubuntu-24.04

eslint.config.mjs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import mocha from "eslint-plugin-mocha";
2+
import promise from "eslint-plugin-promise";
3+
import n from "eslint-plugin-n";
24
import path from "node:path";
35
import { fileURLToPath } from "node:url";
46
import js from "@eslint/js";
@@ -15,6 +17,8 @@ const compat = new FlatCompat({
1517
export default [...compat.extends('@scality/scality'), {
1618
plugins: {
1719
mocha,
20+
promise,
21+
n,
1822
},
1923

2024
languageOptions: {
@@ -67,5 +71,7 @@ export default [...compat.extends('@scality/scality'), {
6771
"quote-props": "off",
6872
"mocha/no-exclusive-tests": "error",
6973
"no-redeclare": ["error", { "builtinGlobals": false }],
74+
"promise/prefer-await-to-then": "warn",
75+
"n/callback-return": "warn",
7076
},
7177
}];

0 commit comments

Comments
 (0)