Skip to content

Commit dbe9006

Browse files
cortinicometa-codesync[bot]
authored andcommitted
Report formatting fixes on pull requests
Summary: Add a GitHub Actions formatting check that runs the repository-wide formatter check and posts line-level suggested fixes through the GitHub API for files that need reformatting. Changelog: [Internal] Differential Revision: D119487616
1 parent 381734f commit dbe9006

5 files changed

Lines changed: 446 additions & 1 deletion

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @format
8+
*/
9+
10+
'use strict';
11+
12+
const {
13+
commentableRightLines,
14+
parsePatch,
15+
} = require('../reportFormattingErrors');
16+
17+
describe('reportFormattingErrors', () => {
18+
test('converts formatter hunks into minimal suggestions', () => {
19+
const patch = `diff --git a/example.js b/example.js
20+
--- a/example.js
21+
+++ b/example.js
22+
@@ -10,3 +10,3 @@
23+
unchanged
24+
-const value={answer:42};
25+
+const value = {answer: 42};
26+
unchanged
27+
`;
28+
29+
expect(parsePatch(patch)).toEqual([
30+
{
31+
path: 'example.js',
32+
startLine: 11,
33+
endLine: 11,
34+
replacement: 'const value = {answer: 42};',
35+
},
36+
]);
37+
});
38+
39+
test('tracks lines that can receive right-side review comments', () => {
40+
const lines = commentableRightLines(`@@ -4,2 +4,3 @@
41+
context
42+
-old
43+
+new
44+
+added
45+
`);
46+
47+
expect([...lines]).toEqual([4, 5, 6]);
48+
});
49+
});
Lines changed: 291 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,291 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @noflow
8+
* @format
9+
*/
10+
11+
'use strict';
12+
13+
const fs = require('node:fs');
14+
const path = require('node:path');
15+
16+
const MARKER = '<!-- react-native-format-report -->';
17+
const MAX_ARTIFACT_BYTES = 1024 * 1024;
18+
const MAX_COMMENTS = 20;
19+
const MAX_REPLACEMENT_LINES = 100;
20+
21+
function readBoundedFile(file) {
22+
const stat = fs.statSync(file);
23+
if (stat.size > MAX_ARTIFACT_BYTES) {
24+
throw new Error(`${path.basename(file)} exceeds the reporting size limit.`);
25+
}
26+
return fs.readFileSync(file, 'utf8');
27+
}
28+
29+
function parsePatch(patch) {
30+
const changes = [];
31+
let file = null;
32+
let hunk = null;
33+
34+
function finishHunk() {
35+
if (file == null || hunk == null) {
36+
return;
37+
}
38+
let prefix = 0;
39+
while (
40+
prefix < hunk.oldLines.length &&
41+
prefix < hunk.newLines.length &&
42+
hunk.oldLines[prefix] === hunk.newLines[prefix]
43+
) {
44+
prefix++;
45+
}
46+
let suffix = 0;
47+
while (
48+
suffix < hunk.oldLines.length - prefix &&
49+
suffix < hunk.newLines.length - prefix &&
50+
hunk.oldLines[hunk.oldLines.length - suffix - 1] ===
51+
hunk.newLines[hunk.newLines.length - suffix - 1]
52+
) {
53+
suffix++;
54+
}
55+
const oldLines = hunk.oldLines.slice(prefix, hunk.oldLines.length - suffix);
56+
const newLines = hunk.newLines.slice(prefix, hunk.newLines.length - suffix);
57+
if (
58+
oldLines.length > 0 &&
59+
oldLines.length <= MAX_REPLACEMENT_LINES &&
60+
newLines.length <= MAX_REPLACEMENT_LINES &&
61+
!newLines.some(line => line.includes('```'))
62+
) {
63+
const startLine = hunk.oldStart + prefix;
64+
changes.push({
65+
path: file,
66+
startLine,
67+
endLine: startLine + oldLines.length - 1,
68+
replacement: newLines.join('\n'),
69+
});
70+
}
71+
hunk = null;
72+
}
73+
74+
for (const line of patch.split('\n')) {
75+
if (line.startsWith('diff --git ')) {
76+
finishHunk();
77+
file = null;
78+
} else if (line.startsWith('+++ b/')) {
79+
const candidate = line.slice(6);
80+
file =
81+
candidate.includes('\0') ||
82+
candidate.split('/').includes('..') ||
83+
path.posix.isAbsolute(candidate)
84+
? null
85+
: candidate;
86+
} else if (line.startsWith('@@ ')) {
87+
finishHunk();
88+
const match = /^@@ -(\d+)(?:,\d+)? \+\d+(?:,\d+)? @@/.exec(line);
89+
hunk =
90+
match == null
91+
? null
92+
: {oldStart: Number(match[1]), oldLines: [], newLines: []};
93+
} else if (hunk != null && !line.startsWith('\\ No newline')) {
94+
if (!line.startsWith('+')) {
95+
hunk.oldLines.push(line.slice(1));
96+
}
97+
if (!line.startsWith('-')) {
98+
hunk.newLines.push(line.slice(1));
99+
}
100+
}
101+
}
102+
finishHunk();
103+
return changes;
104+
}
105+
106+
function commentableRightLines(patch) {
107+
const lines = new Set();
108+
let newLine = 0;
109+
for (const line of (patch ?? '').split('\n')) {
110+
const match = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line);
111+
if (match != null) {
112+
newLine = Number(match[1]);
113+
} else if (line.startsWith('+') || line.startsWith(' ')) {
114+
lines.add(newLine++);
115+
} else if (
116+
!line.startsWith('-') &&
117+
!line.startsWith('\\ No newline') &&
118+
newLine !== 0
119+
) {
120+
newLine++;
121+
}
122+
}
123+
return lines;
124+
}
125+
126+
async function deletePreviousComments(github, owner, repo, pullNumber) {
127+
const issueComments = await github.paginate(github.rest.issues.listComments, {
128+
owner,
129+
repo,
130+
issue_number: pullNumber,
131+
per_page: 100,
132+
});
133+
for (const comment of issueComments) {
134+
if (
135+
comment.user?.login === 'github-actions[bot]' &&
136+
comment.body?.includes(MARKER)
137+
) {
138+
await github.rest.issues.deleteComment({
139+
owner,
140+
repo,
141+
comment_id: comment.id,
142+
});
143+
}
144+
}
145+
146+
const reviewComments = await github.paginate(
147+
github.rest.pulls.listReviewComments,
148+
{owner, repo, pull_number: pullNumber, per_page: 100},
149+
);
150+
for (const comment of reviewComments) {
151+
if (
152+
comment.user?.login === 'github-actions[bot]' &&
153+
comment.body?.includes(MARKER)
154+
) {
155+
await github.rest.pulls.deleteReviewComment({
156+
owner,
157+
repo,
158+
comment_id: comment.id,
159+
});
160+
}
161+
}
162+
}
163+
164+
module.exports = async function reportFormattingErrors({
165+
github,
166+
context,
167+
core,
168+
}) {
169+
const run = context.payload.workflow_run;
170+
const pullRequests = run.pull_requests ?? [];
171+
if (pullRequests.length !== 1) {
172+
core.warning('Expected exactly one pull request for the formatting run.');
173+
return;
174+
}
175+
176+
const metadata = JSON.parse(readBoundedFile('.format-results/metadata.json'));
177+
const pullNumber = Number(metadata.PR_NUMBER);
178+
const headSha = metadata.HEAD_SHA;
179+
if (
180+
metadata.EVENT_NAME !== 'pull_request' ||
181+
!Number.isSafeInteger(pullNumber) ||
182+
pullNumber !== pullRequests[0].number ||
183+
!/^[0-9a-f]{40}$/.test(headSha)
184+
) {
185+
throw new Error('Formatting artifact metadata does not match this run.');
186+
}
187+
188+
const {owner, repo} = context.repo;
189+
const {data: pullRequest} = await github.rest.pulls.get({
190+
owner,
191+
repo,
192+
pull_number: pullNumber,
193+
});
194+
if (pullRequest.head.sha !== headSha) {
195+
core.info(
196+
'Ignoring a stale formatting result for an older pull request revision.',
197+
);
198+
return;
199+
}
200+
201+
await deletePreviousComments(github, owner, repo, pullNumber);
202+
if (run.conclusion === 'success') {
203+
return;
204+
}
205+
206+
const patchFile = '.format-results/format.patch';
207+
const patch = fs.existsSync(patchFile) ? readBoundedFile(patchFile) : '';
208+
const outputFile = '.format-results/output.txt';
209+
const output = fs.existsSync(outputFile) ? readBoundedFile(outputFile) : '';
210+
const files = await github.paginate(github.rest.pulls.listFiles, {
211+
owner,
212+
repo,
213+
pull_number: pullNumber,
214+
per_page: 100,
215+
});
216+
const pullPatches = new Map(files.map(file => [file.filename, file.patch]));
217+
const suggestions = parsePatch(patch)
218+
.filter(change => {
219+
const commentable = commentableRightLines(pullPatches.get(change.path));
220+
return (
221+
commentable.has(change.startLine) && commentable.has(change.endLine)
222+
);
223+
})
224+
.slice(0, MAX_COMMENTS);
225+
226+
let postedSuggestions = 0;
227+
for (const suggestion of suggestions) {
228+
try {
229+
const location =
230+
suggestion.startLine === suggestion.endLine
231+
? {}
232+
: {
233+
start_line: suggestion.startLine,
234+
start_side: 'RIGHT',
235+
};
236+
await github.rest.pulls.createReviewComment({
237+
owner,
238+
repo,
239+
pull_number: pullNumber,
240+
commit_id: headSha,
241+
path: suggestion.path,
242+
...location,
243+
line: suggestion.endLine,
244+
side: 'RIGHT',
245+
body: `${MARKER}\n\`yarn format\` suggests:\n\n\`\`\`suggestion\n${suggestion.replacement}\n\`\`\``,
246+
});
247+
postedSuggestions++;
248+
} catch (error) {
249+
core.warning(
250+
`Could not attach a suggestion to ${suggestion.path}: ${error}`,
251+
);
252+
}
253+
}
254+
255+
const changedFiles = [
256+
...new Set(parsePatch(patch).map(change => change.path)),
257+
];
258+
const details =
259+
changedFiles.length > 0
260+
? changedFiles.map(file => `- \`${file}\``).join('\n')
261+
: 'The formatter stopped before producing a patch. See the workflow log.';
262+
const outputExcerpt = output
263+
.slice(-4000)
264+
.replaceAll('```', '``\\`')
265+
.replaceAll('<', '&lt;');
266+
const body = `${MARKER}
267+
## Formatting required
268+
269+
Run \`yarn format\` from the repository root and commit the result.
270+
271+
${details}
272+
273+
${postedSuggestions} inline suggestion${postedSuggestions === 1 ? '' : 's'} posted. Suggestions can only be attached to lines visible in the pull request diff.
274+
275+
<details><summary>Formatter output</summary>
276+
277+
\`\`\`text
278+
${outputExcerpt}
279+
\`\`\`
280+
</details>`;
281+
282+
await github.rest.issues.createComment({
283+
owner,
284+
repo,
285+
issue_number: pullNumber,
286+
body,
287+
});
288+
};
289+
290+
module.exports.parsePatch = parsePatch;
291+
module.exports.commentableRightLines = commentableRightLines;
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
name: Format Report
2+
3+
on:
4+
workflow_run:
5+
workflows: [Format]
6+
types: [completed]
7+
8+
permissions:
9+
actions: read
10+
contents: read
11+
issues: write
12+
pull-requests: write
13+
14+
jobs:
15+
report:
16+
runs-on: ubuntu-latest
17+
if: >-
18+
github.repository == 'react/react-native' &&
19+
github.event.workflow_run.event == 'pull_request'
20+
steps:
21+
- name: Check out trusted reporter
22+
uses: actions/checkout@v6
23+
- name: Download formatting report
24+
uses: actions/download-artifact@v7
25+
with:
26+
name: format-results
27+
path: .format-results
28+
run-id: ${{ github.event.workflow_run.id }}
29+
github-token: ${{ github.token }}
30+
- name: Comment on formatting failures
31+
uses: actions/github-script@v8
32+
with:
33+
script: |
34+
const reportFormattingErrors = require('./.github/workflow-scripts/reportFormattingErrors');
35+
await reportFormattingErrors({github, context, core});

0 commit comments

Comments
 (0)