Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit defdb6d

Browse files
nganbreadfacebook-github-bot
authored andcommittedDec 7, 2020
Add Java linting using google-java-format (#30444)
Summary: Adds `google-java-format` linting for all `.java` files in the `ReactAndroid/` folder - Linting requires java and is now performed on the android container - https://github.com/google/google-java-format ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [Internal] [Added] - Linting for *.java files (google-java-format) Pull Request resolved: #30444 Test Plan: See this example PR for lint comments: #30512 Reviewed By: hramos Differential Revision: D25253627 Pulled By: nganbread fbshipit-source-id: e39e4411bf09a96c054afaf6c12b3d05a80f40fa
1 parent 503a6f4 commit defdb6d

File tree

6 files changed

+319
-85
lines changed

6 files changed

+319
-85
lines changed
 

‎.circleci/config.yml

+29-15
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ defaults: &defaults
1515
environment:
1616
- GIT_COMMIT_DESC: git log --format=oneline -n 1 $CIRCLE_SHA1
1717
# The public github tokens are publicly visible by design
18-
- PUBLIC_PULLBOT_GITHUB_TOKEN_A: "a6edf8e8d40ce4e8b11a"
19-
- PUBLIC_PULLBOT_GITHUB_TOKEN_B: "150e1341f4dd9c944d2a"
20-
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A: &github_token_a "78a72af35445ca3f8180"
21-
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B: &github_token_b "b1a98e0bbd56ff1ccba1"
18+
- PUBLIC_PULLBOT_GITHUB_TOKEN_A: &github_pullbot_token_a "a6edf8e8d40ce4e8b11a"
19+
- PUBLIC_PULLBOT_GITHUB_TOKEN_B: &github_pullbot_token_b "150e1341f4dd9c944d2a"
20+
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A: &github_analysisbot_token_a "312d354b5c36f082cfe9"
21+
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B: &github_analysisbot_token_b "07973d757026bdd9f196"
2222

2323
# -------------------------
2424
# EXECUTORS
@@ -44,8 +44,10 @@ executors:
4444
- GRADLE_OPTS: '-Dorg.gradle.daemon=false -Dorg.gradle.jvmargs="-XX:+HeapDumpOnOutOfMemoryError"'
4545
- BUILD_THREADS: 2
4646
# Repeated here, as the environment key in this executor will overwrite the one in defaults
47-
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A: *github_token_a
48-
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B: *github_token_b
47+
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A: *github_analysisbot_token_a
48+
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B: *github_analysisbot_token_b
49+
- PUBLIC_PULLBOT_GITHUB_TOKEN_A: *github_pullbot_token_a
50+
- PUBLIC_PULLBOT_GITHUB_TOKEN_B: *github_pullbot_token_b
4951
reactnativeios:
5052
<<: *defaults
5153
macos:
@@ -234,17 +236,17 @@ jobs:
234236
# Issues will be posted to the PR itself via GitHub bots.
235237
# This workflow should only fail if the bots fail to run.
236238
analyze_pr:
237-
executor: nodelts
239+
executor: reactnativeandroid
238240
steps:
239241
- restore_cache_checkout:
240-
checkout_type: node
242+
checkout_type: android
241243
- run_yarn
242244

243245
- install_github_bot_deps
244246

245247
- run:
246248
name: Install additional GitHub bot dependencies
247-
command: sudo apt update && sudo apt install -y shellcheck jq
249+
command: apt update && apt install -y shellcheck jq
248250

249251
- run:
250252
name: Run linters against modified files (analysis-bot)
@@ -262,10 +264,10 @@ jobs:
262264
# JOBS: Analyze Code
263265
# -------------------------
264266
analyze_code:
265-
executor: nodelts
267+
executor: reactnativeandroid
266268
steps:
267269
- restore_cache_checkout:
268-
checkout_type: node
270+
checkout_type: android
269271
- setup_artifacts
270272
- run_yarn
271273

@@ -274,6 +276,11 @@ jobs:
274276
command: scripts/circleci/exec_swallow_error.sh yarn lint --format junit -o ./reports/junit/eslint/results.xml
275277
when: always
276278

279+
- run:
280+
name: Lint Java
281+
command: scripts/circleci/exec_swallow_error.sh yarn lint-java --check
282+
when: always
283+
277284
- run:
278285
name: Check for errors in code using Flow (iOS)
279286
command: yarn flow-check-ios
@@ -847,27 +854,34 @@ workflows:
847854

848855
analysis:
849856
jobs:
850-
- setup
857+
- setup:
858+
name: setup_js
859+
860+
- setup:
861+
name: setup_android
862+
checkout_type: android
863+
executor: reactnativeandroid
864+
851865
# Run lints on every commit other than those to the gh-pages branch
852866
- analyze_code:
853867
requires:
854-
- setup
868+
- setup_android
855869
filters:
856870
branches:
857871
ignore: gh-pages
858872

859873
# Run code checks on PRs from forks
860874
- analyze_pr:
861875
requires:
862-
- setup
876+
- setup_android
863877
filters:
864878
branches:
865879
only: /^pull\/.*$/
866880

867881
# Gather coverage
868882
- js_coverage:
869883
requires:
870-
- setup
884+
- setup_js
871885
nightly:
872886
triggers:
873887
- schedule:

‎bots/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ DANGER_GITHUB_API_TOKEN=[ENV_ABOVE] yarn danger pr https://github.com/facebook/r
1515
The code analysis bot provides lint and other results as inline reviews on GitHub. It runs as part of the Circle CI analysis workflow.
1616

1717
If you want to test changes to the Code Analysis Bot, I'd recommend checking out an existing PR and then running the `analyze pr` command.
18-
You'll need a GitHub token. You can re-use this one: `78a72af35445ca3f8180` `b1a98e0bbd56ff1ccba1` (just remove the space).
18+
You'll need a GitHub token. You can re-use this one: `312d354b5c36f082cfe9` `07973d757026bdd9f196` (just remove the space).
1919
So, for example:
2020

2121
```

‎bots/code-analysis-bot.js

+82-68
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ const converterSummary = {
3434
'`flow` found some issues. Run `yarn flow check` to analyze your code and address any errors.',
3535
shellcheck:
3636
'`shellcheck` found some issues. Run `yarn shellcheck` to analyze shell scripts.',
37+
'google-java-format':
38+
'`google-java-format` found some issues. See https://github.com/google/google-java-format',
3739
};
3840

3941
/**
@@ -57,6 +59,24 @@ const converters = {
5759
}
5860
},
5961

62+
'google-java-format': function(output, input) {
63+
if (!input) {
64+
return;
65+
}
66+
67+
input.forEach(function(change) {
68+
push(output, change.file, {
69+
message: `\`google-java-format\` suggested changes:
70+
\`\`\`diff
71+
${change.description}
72+
\`\`\`
73+
`,
74+
line: change.line,
75+
converter: 'google-java-format',
76+
});
77+
});
78+
},
79+
6080
flow: function(output, input) {
6181
if (!input || !input.errors) {
6282
return;
@@ -113,30 +133,6 @@ const converters = {
113133
},
114134
};
115135

116-
function getShaFromPullRequest(octokit, owner, repo, number, callback) {
117-
octokit.pullRequests.get({owner, repo, number}, (error, res) => {
118-
if (error) {
119-
console.error(error);
120-
return;
121-
}
122-
123-
callback(res.data.head.sha);
124-
});
125-
}
126-
127-
function getFilesFromPullRequest(octokit, owner, repo, number, callback) {
128-
octokit.pullRequests.listFiles(
129-
{owner, repo, number, per_page: 100},
130-
(error, res) => {
131-
if (error) {
132-
console.error(error);
133-
return;
134-
}
135-
callback(res.data);
136-
},
137-
);
138-
}
139-
140136
/**
141137
* Sadly we can't just give the line number to github, we have to give the
142138
* line number relative to the patch file which is super annoying. This
@@ -166,7 +162,15 @@ function getLineMapFromPatch(patchString) {
166162
return lineMap;
167163
}
168164

169-
function sendReview(octokit, owner, repo, number, commit_id, body, comments) {
165+
async function sendReview(
166+
octokit,
167+
owner,
168+
repo,
169+
pull_number,
170+
commit_id,
171+
body,
172+
comments,
173+
) {
170174
if (process.env.GITHUB_TOKEN) {
171175
if (comments.length === 0) {
172176
// Do not leave an empty review.
@@ -181,19 +185,14 @@ function sendReview(octokit, owner, repo, number, commit_id, body, comments) {
181185
const opts = {
182186
owner,
183187
repo,
184-
number,
188+
pull_number,
185189
commit_id,
186190
body,
187191
event,
188192
comments,
189193
};
190194

191-
octokit.pullRequests.createReview(opts, function(error, res) {
192-
if (error) {
193-
console.error(error);
194-
return;
195-
}
196-
});
195+
await octokit.pulls.createReview(opts);
197196
} else {
198197
if (comments.length === 0) {
199198
console.log('No issues found.');
@@ -216,7 +215,7 @@ function sendReview(octokit, owner, repo, number, commit_id, body, comments) {
216215
}
217216
}
218217

219-
function main(messages, owner, repo, number) {
218+
async function main(messages, owner, repo, pull_number) {
220219
// No message, we don't need to do anything :)
221220
if (Object.keys(messages).length === 0) {
222221
return;
@@ -234,40 +233,54 @@ function main(messages, owner, repo, number) {
234233
auth: process.env.GITHUB_TOKEN,
235234
});
236235

237-
getShaFromPullRequest(octokit, owner, repo, number, sha => {
238-
getFilesFromPullRequest(octokit, owner, repo, number, files => {
239-
let comments = [];
240-
let convertersUsed = [];
241-
files
242-
.filter(file => messages[file.filename])
243-
.forEach(file => {
244-
// github api sometimes does not return a patch on large commits
245-
if (!file.patch) {
246-
return;
247-
}
248-
const lineMap = getLineMapFromPatch(file.patch);
249-
messages[file.filename].forEach(message => {
250-
if (lineMap[message.line]) {
251-
const comment = {
252-
path: file.filename,
253-
position: lineMap[message.line],
254-
body: message.message,
255-
};
256-
convertersUsed.push(message.converter);
257-
comments.push(comment);
258-
}
259-
}); // forEach
260-
}); // filter
261-
262-
let body = '**Code analysis results:**\n\n';
263-
const uniqueconvertersUsed = [...new Set(convertersUsed)];
264-
uniqueconvertersUsed.forEach(converter => {
265-
body += '* ' + converterSummary[converter] + '\n';
266-
});
236+
const opts = {
237+
owner,
238+
repo,
239+
pull_number,
240+
};
267241

268-
sendReview(octokit, owner, repo, number, sha, body, comments);
269-
}); // getFilesFromPullRequest
270-
}); // getShaFromPullRequest
242+
const {data: pull} = await octokit.pulls.get(opts);
243+
const {data: files} = await octokit.pulls.listFiles(opts);
244+
245+
const comments = [];
246+
const convertersUsed = [];
247+
248+
files
249+
.filter(file => messages[file.filename])
250+
.forEach(file => {
251+
// github api sometimes does not return a patch on large commits
252+
if (!file.patch) {
253+
return;
254+
}
255+
const lineMap = getLineMapFromPatch(file.patch);
256+
messages[file.filename].forEach(message => {
257+
if (lineMap[message.line]) {
258+
const comment = {
259+
path: file.filename,
260+
position: lineMap[message.line],
261+
body: message.message,
262+
};
263+
convertersUsed.push(message.converter);
264+
comments.push(comment);
265+
}
266+
}); // forEach
267+
}); // filter
268+
269+
let body = '**Code analysis results:**\n\n';
270+
const uniqueconvertersUsed = [...new Set(convertersUsed)];
271+
uniqueconvertersUsed.forEach(converter => {
272+
body += '* ' + converterSummary[converter] + '\n';
273+
});
274+
275+
await sendReview(
276+
octokit,
277+
owner,
278+
repo,
279+
pull_number,
280+
pull.head.sha,
281+
body,
282+
comments,
283+
);
271284
}
272285

273286
let content = '';
@@ -331,6 +344,7 @@ process.stdin.on('end', function() {
331344

332345
const number = process.env.GITHUB_PR_NUMBER;
333346

334-
// intentional lint warning to make sure that the bot is working :)
335-
main(messages, owner, repo, number);
347+
(async () => {
348+
await main(messages, owner, repo, number);
349+
})();
336350
});

‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
"flow-check-android": "flow check --flowconfig-name .flowconfig.android",
6161
"lint": "eslint .",
6262
"lint-ci": "./scripts/circleci/analyze_code.sh && yarn shellcheck",
63+
"lint-java": "node ./scripts/lint-java.js",
6364
"shellcheck": "./scripts/circleci/analyze_scripts.sh",
6465
"clang-format": "clang-format -i --glob=*/**/*.{h,cpp,m,mm}",
6566
"format": "npm run prettier && npm run clang-format",

‎scripts/circleci/analyze_code.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ GITHUB_REPO=${CIRCLE_PROJECT_REPONAME:-react-native}
99
export GITHUB_OWNER
1010
export GITHUB_REPO
1111

12-
cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow-check-ios --silent --json; echo flow; npm run flow-check-android --silent --json) | GITHUB_PR_NUMBER="$CIRCLE_PR_NUMBER" node bots/code-analysis-bot.js
12+
cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow-check-ios --silent --json; echo flow; npm run flow-check-android --silent --json; echo google-java-format; node scripts/lint-java.js --diff) | GITHUB_PR_NUMBER="$CIRCLE_PR_NUMBER" node bots/code-analysis-bot.js
1313

1414
STATUS=$?
1515
if [ $STATUS == 0 ]; then

0 commit comments

Comments
 (0)
Please sign in to comment.