diff --git a/.github/actions/lint/action.yml b/.github/actions/lint/action.yml index 1073484039e9..35e4f70f84f4 100644 --- a/.github/actions/lint/action.yml +++ b/.github/actions/lint/action.yml @@ -5,9 +5,6 @@ inputs: description: "The node.js version to use" required: false default: "22" - github-token: - description: "The GitHub token used by pull-bot" - required: true runs: using: composite steps: @@ -17,12 +14,9 @@ runs: node-version: ${{ inputs.node-version }} - name: Run yarn install uses: ./.github/actions/yarn-install - - name: Run linters against modified files (analysis-bot) + - name: Run shellcheck shell: bash - run: yarn lint-ci - env: - GITHUB_TOKEN: ${{ inputs.github-token }} - GITHUB_PR_NUMBER: ${{ github.event.number }} + run: yarn shellcheck - name: Lint code shell: bash run: ./.github/workflow-scripts/exec_swallow_error.sh yarn lint --format junit -o ./reports/junit/eslint/results.xml diff --git a/.github/workflow-scripts/analyze_code.sh b/.github/workflow-scripts/analyze_code.sh deleted file mode 100755 index 8ea078174c1f..000000000000 --- a/.github/workflow-scripts/analyze_code.sh +++ /dev/null @@ -1,24 +0,0 @@ -#!/bin/bash -# Copyright (c) Meta Platforms, Inc. and affiliates. -# -# This source code is licensed under the MIT license found in the -# LICENSE file in the root directory of this source tree. - -export GITHUB_OWNER=-facebook -export GITHUB_REPO=-react-native - -{ - echo eslint - npm run lint --silent -- --format=json - - echo flow - npm run flow-check --silent --json -} | node private/react-native-bots/code-analysis-bot.js - -STATUS=$? -if [ $STATUS == 0 ]; then - echo "Code analyzed successfully." -else - echo "Code analysis failed, error status $STATUS." -fi -exit $STATUS diff --git a/.github/workflows/test-all.yml b/.github/workflows/test-all.yml index 5a59c493f41e..3e78c4620e07 100644 --- a/.github/workflows/test-all.yml +++ b/.github/workflows/test-all.yml @@ -33,6 +33,23 @@ jobs: echo "Should I run E2E tests? ${{ inputs.run-e2e-tests }}" + check_code_changes: + runs-on: ubuntu-latest + if: github.repository == 'facebook/react-native' + outputs: + debugger_shell: ${{ steps.filter.outputs.debugger_shell }} + steps: + - name: Checkout + uses: actions/checkout@v6 + - name: Check for code changes + uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 + id: filter + with: + filters: | + debugger_shell: + - 'packages/debugger-shell/**' + - 'scripts/debugger-shell/**' + prebuild_apple_dependencies: if: github.repository == 'facebook/react-native' uses: ./.github/workflows/prebuild-ios-dependencies.yml @@ -494,8 +511,22 @@ jobs: uses: actions/checkout@v6 - name: Run all the linters uses: ./.github/actions/lint - with: - github-token: ${{ env.GH_TOKEN }} + + build_debugger_shell: + runs-on: ubuntu-latest + needs: check_code_changes + if: needs.check_code_changes.outputs.debugger_shell == 'true' + steps: + - name: Checkout + uses: actions/checkout@v6 + - name: Setup node.js + uses: ./.github/actions/setup-node + - name: Run yarn install + uses: ./.github/actions/yarn-install + - name: Build packages + run: yarn build + - name: Verify debugger-shell build + run: node scripts/debugger-shell/build-binary.js # This job should help with the E2E flakyness. # In case E2E tests fails, it launches a new retry-workflow workflow, passing the current run_id as input. diff --git a/package.json b/package.json index fef5a3172a1b..99d311f02a47 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,6 @@ "format": "npm run prettier && npm run clang-format", "featureflags": "yarn --cwd packages/react-native featureflags", "js-api-diff": "node ./scripts/js-api/diff-api-snapshot", - "lint-ci": "./.github/workflow-scripts/analyze_code.sh && yarn shellcheck", "lint-kotlin-check": "./gradlew ktfmtCheck", "lint-kotlin": "./gradlew ktfmtFormat", "lint-markdown": "markdownlint-cli2 2>&1", diff --git a/private/react-native-bots/README.md b/private/react-native-bots/README.md index 3848beb5d90a..5446d108d561 100644 --- a/private/react-native-bots/README.md +++ b/private/react-native-bots/README.md @@ -12,15 +12,3 @@ So, for example: cd private/react-native-bots && yarn DANGER_GITHUB_API_TOKEN=ghp_ yarn danger pr https://github.com/facebook/react-native/pull/1234 ``` - -## Code Analysis Bot - -The code analysis bot provides lint and other results as inline reviews on GitHub. It runs as part of the Circle CI analysis workflow. - -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. -You'll need a GitHub token. You can re-use this one: `312d354b5c36f082cfe9` `07973d757026bdd9f196` (just remove the space). -So, for example: - -``` -GITHUB_TOKEN=[ENV_ABOVE] GITHUB_PR_NUMBER=1234 yarn lint-ci -``` diff --git a/private/react-native-bots/code-analysis-bot.js b/private/react-native-bots/code-analysis-bot.js deleted file mode 100644 index 258e68798245..000000000000 --- a/private/react-native-bots/code-analysis-bot.js +++ /dev/null @@ -1,344 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @format - * @noflow - */ - -'use strict'; - -if (!process.env.GITHUB_OWNER) { - console.error('Missing GITHUB_OWNER. Example: facebook'); - process.exit(1); -} -if (!process.env.GITHUB_REPO) { - console.error('Missing GITHUB_REPO. Example: react-native'); - process.exit(1); -} - -const path = require('path'); - -function push(arr, key, value) { - if (!arr[key]) { - arr[key] = []; - } - arr[key].push(value); -} - -const converterSummary = { - eslint: - '`eslint` found some issues. Run `yarn lint --fix` to automatically fix problems.', - flow: '`flow` found some issues. Run `yarn flow check` to analyze your code and address any errors.', - shellcheck: - '`shellcheck` found some issues. Run `yarn shellcheck` to analyze shell scripts.', - 'google-java-format': - '`google-java-format` found some issues. See https://github.com/google/google-java-format', -}; - -/** - * There is unfortunately no standard format to report an error, so we have - * to write a specific converter for each tool we want to support. - * - * Those functions take a json object as input and fill the output with the - * following format: - * - * { [ path: string ]: Array< { message: string, line: number }> } - * - * This is an object where the keys are the path of the files and values - * is an array of objects of the shape message and line. - */ -const converters = { - raw: function (output, input) { - for (let key in input) { - input[key].forEach(function (message) { - push(output, key, message); - }); - } - }, - - 'google-java-format': function (output, input) { - if (!input) { - return; - } - - input.forEach(function (change) { - push(output, change.file, { - message: `\`google-java-format\` suggested changes: -\`\`\`diff -${change.description} -\`\`\` -`, - line: change.line, - converter: 'google-java-format', - }); - }); - }, - - flow: function (output, input) { - if (!input || !input.errors) { - return; - } - - input.errors.forEach(function (error) { - push(output, error.message[0].path, { - message: error.message.map(message => message.descr).join(' '), - line: error.message[0].line, - converter: 'flow', - }); - }); - }, - - eslint: function (output, input) { - if (!input) { - return; - } - - input.forEach(function (file) { - file.messages.forEach(function (message) { - push(output, file.filePath, { - message: message.ruleId + ': ' + message.message, - line: message.line, - converter: 'eslint', - }); - }); - }); - }, - - shellcheck: function (output, input) { - if (!input) { - return; - } - - input.forEach(function (report) { - push(output, report.file, { - message: - '**[SC' + - report.code + - '](https://github.com/koalaman/shellcheck/wiki/SC' + - report.code + - '):** (' + - report.level + - ') ' + - report.message, - line: report.line, - endLine: report.endLine, - column: report.column, - endColumn: report.endColumn, - converter: 'shellcheck', - }); - }); - }, -}; - -/** - * Sadly we can't just give the line number to github, we have to give the - * line number relative to the patch file which is super annoying. This - * little function builds a map of line number in the file to line number - * in the patch file - */ -function getLineMapFromPatch(patchString) { - let diffLineIndex = 0; - let fileLineIndex = 0; - let lineMap = {}; - - patchString.split('\n').forEach(line => { - if (line.match(/^@@/)) { - fileLineIndex = line.match(/\+([0-9]+)/)[1] - 1; - return; - } - - diffLineIndex++; - if (line[0] !== '-') { - fileLineIndex++; - if (line[0] === '+') { - lineMap[fileLineIndex] = diffLineIndex; - } - } - }); - - return lineMap; -} - -async function sendReview( - octokit, - owner, - repo, - pull_number, - commit_id, - body, - comments, -) { - if (process.env.GITHUB_TOKEN) { - if (comments.length === 0) { - // Do not leave an empty review. - return; - } else if (comments.length > 5) { - // Avoid noisy reviews and rely solely on the body of the review. - comments = []; - } - - const event = 'REQUEST_CHANGES'; - - const opts = { - owner, - repo, - pull_number, - commit_id, - body, - event, - comments, - }; - - await octokit.pulls.createReview(opts); - } else { - if (comments.length === 0) { - console.log('No issues found.'); - return; - } - - let results = body + '\n'; - comments.forEach(comment => { - results += - comment.path + ':' + comment.position + ': ' + comment.body + '\n'; - }); - console.log(results); - } -} - -async function main(messages, owner, repo, pull_number) { - // No message, we don't need to do anything :) - if (Object.keys(messages).length === 0) { - return; - } - - if (!process.env.GITHUB_TOKEN) { - console.log( - 'Missing GITHUB_TOKEN. Example: 5fd88b964fa214c4be2b144dc5af5d486a2f8c1e. Review feedback with code analysis results will not be provided on GitHub without a valid token.', - ); - } - - // https://octokit.github.io/rest.js/ - const {Octokit} = require('@octokit/rest'); - const octokit = new Octokit({ - auth: process.env.GITHUB_TOKEN, - userAgent: 'react-native-code-analysis-bot', - }); - - const opts = { - owner, - repo, - pull_number, - }; - - const {data: pull} = await octokit.pulls.get(opts); - const {data: files} = await octokit.pulls.listFiles(opts); - - const comments = []; - const convertersUsed = []; - - files - .filter(file => messages[file.filename]) - .forEach(file => { - // github api sometimes does not return a patch on large commits - if (!file.patch) { - return; - } - const lineMap = getLineMapFromPatch(file.patch); - messages[file.filename].forEach(message => { - if (lineMap[message.line]) { - const comment = { - path: file.filename, - position: lineMap[message.line], - body: message.message, - }; - convertersUsed.push(message.converter); - comments.push(comment); - } - }); // forEach - }); // filter - - let body = '**Code analysis results:**\n\n'; - const uniqueconvertersUsed = [...new Set(convertersUsed)]; - uniqueconvertersUsed.forEach(converter => { - body += '* ' + converterSummary[converter] + '\n'; - }); - - await sendReview( - octokit, - owner, - repo, - pull_number, - pull.head.sha, - body, - comments, - ); -} - -let content = ''; -process.stdin.resume(); -process.stdin.on('data', function (buf) { - content += buf.toString(); -}); -process.stdin.on('end', function () { - let messages = {}; - - // Since we send a few http requests to setup the process, we don't want - // to run this file one time per code analysis tool. Instead, we write all - // the results in the same stdin stream. - // The format of this stream is - // - // name-of-the-converter - // {"json":"payload"} - // name-of-the-other-converter - // {"other": ["json", "payload"]} - // - // In order to generate such stream, here is a sample bash command: - // - // cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; flow --json) | node code-analysis-bot.js - - const lines = content.trim().split('\n'); - for (let i = 0; i < Math.ceil(lines.length / 2); ++i) { - const converter = converters[lines[i * 2]]; - if (!converter) { - throw new Error('Unknown converter ' + lines[i * 2]); - } - let json; - try { - json = JSON.parse(lines[i * 2 + 1]); - } catch (e) {} - - converter(messages, json); - } - - // The paths are returned in absolute from code analysis tools but github works - // on paths relative from the root of the project. Doing the normalization here. - const pwd = path.resolve('.'); - for (let absolutePath in messages) { - const relativePath = path.relative(pwd, absolutePath); - if (relativePath === absolutePath) { - continue; - } - messages[relativePath] = messages[absolutePath]; - delete messages[absolutePath]; - } - - const owner = process.env.GITHUB_OWNER; - const repo = process.env.GITHUB_REPO; - - if (!process.env.GITHUB_PR_NUMBER) { - console.error( - 'Missing GITHUB_PR_NUMBER. Example: 4687. Review feedback with code analysis results cannot be provided on GitHub without a valid pull request number.', - ); - // for master branch, don't throw an error - process.exit(0); - } - - const number = process.env.GITHUB_PR_NUMBER; - - (async () => { - await main(messages, owner, repo, number); - })(); -});