diff --git a/.circleci/config.yml b/.circleci/config.yml index 3cab7fbadb58..d139db488edf 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -923,25 +923,6 @@ jobs: command: test/docsCodeStyle.sh - matrix_notify_failure_unless_pr - chk_coding_style: - <<: *base_cimg_small - steps: - - checkout - - run: - name: Install shellcheck - command: | - sudo apt -q update - sudo apt install -y shellcheck - - run: - name: Check for C++ coding style - command: scripts/check_style.sh - - run: - name: checking shell scripts - command: scripts/chk_shellscripts/chk_shellscripts.sh - - run: - name: Check for broken symlinks - command: scripts/check_symlinks.sh - - matrix_notify_failure_unless_pr chk_errorcodes: <<: *base_ubuntu2404_small @@ -1997,7 +1978,6 @@ workflows: jobs: # basic checks - chk_spelling: *requires_nothing - - chk_coding_style: *requires_nothing # DISABLED FOR 0.6.0 - chk_docs_examples: *requires_nothing - chk_buglist: *requires_nothing - chk_proofs: *requires_nothing diff --git a/.github/workflows/code-style.yml b/.github/workflows/code-style.yml new file mode 100644 index 000000000000..17347e6d8cfd --- /dev/null +++ b/.github/workflows/code-style.yml @@ -0,0 +1,81 @@ +name: Code Style Check + +on: + pull_request: + types: [opened, synchronize, reopened] + +permissions: + pull-requests: write + contents: read + +jobs: + chk_coding_style: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 #v5.0.0 + + - name: Install dependencies + run: | + sudo apt -q update + sudo apt install -y shellcheck git openssl + + - name: Check for C++ coding style + id: style-check + run: | + output=$(scripts/check_style.sh 2>&1) || { + delimiter="$(openssl rand -hex 8)" + { + echo "output<<${delimiter}" + echo "$output" + echo "${delimiter}" + } >> $GITHUB_OUTPUT + exit 1 + } + + - name: Comment PR on failure + if: ${{ failure() && steps.style-check.outcome == 'failure' }} + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd #v8.0.0 + env: + STYLE_OUTPUT: ${{ steps.style-check.outputs.output }} + with: + script: | + const output = process.env.STYLE_OUTPUT; + const headSha = context.payload.pull_request.head.sha; + const body = `There was an error when running \`Code Style Check\` for commit \`${headSha}\`: + \`\`\` + ${output} + \`\`\` + Please check that your changes are working as intended.`; + + await github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: body + }); + + for (const line of output.split('\n')) { + const match = line.match(/^\[([^\]]+)\]\s*([^:]+\.(cpp|h)):(\d+):/); + if (!match) continue; + const [, errorDescription, filePath, , lineNumber] = match; + try { + await github.rest.pulls.createReviewComment({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.issue.number, + commit_id: headSha, + path: filePath, + line: parseInt(lineNumber), + side: 'RIGHT', + body: `**Coding style error:** ${errorDescription}` + }); + } catch (error) { + console.log(`Could not add inline comment for ${filePath}:${lineNumber}: ${error.message}`); + } + } + + - name: checking shell scripts + run: scripts/chk_shellscripts/chk_shellscripts.sh + + - name: Check for broken symlinks + run: scripts/check_symlinks.sh diff --git a/libsolutil/TestStyleViolations.cpp b/libsolutil/TestStyleViolations.cpp new file mode 100644 index 000000000000..9dbd32165389 --- /dev/null +++ b/libsolutil/TestStyleViolations.cpp @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-3.0 +#include "libsolutil/Common.h" + +using namespace std; + +namespace solidity::util { + +const int BAD_CONST = 42; + +void testFunction() +{ + if(true) { + int& badRef = BAD_CONST; + auto x = move(badRef); + } +} + +} diff --git a/scripts/check_style.sh b/scripts/check_style.sh index d67f7b60ca22..538f20991d72 100755 --- a/scripts/check_style.sh +++ b/scripts/check_style.sh @@ -33,8 +33,6 @@ if [[ "$WHITESPACE" != "" ]] then echo "Error: Trailing whitespace found:" | tee -a "$ERROR_LOG" echo "$WHITESPACE" | tee -a "$ERROR_LOG" - scripts/ci/post_style_errors_on_github.sh "$ERROR_LOG" - exit 1 fi function preparedGrep @@ -43,23 +41,26 @@ function preparedGrep return $? } +function addPrefix +{ + sed "s/^/[$1] /" +} + FORMATERROR=$( ( - preparedGrep "#include \"" | grep -E -v -e "license.h" -e "BuildInfo.h" # Use include with <> characters - preparedGrep "\<(if|for|while|switch)\(" # no space after "if", "for", "while" or "switch" - preparedGrep "\\s*\([^=]*\>\s:\s.*\)" # no space before range based for-loop - preparedGrep "\\s*\(.*\)\s*\{\s*$" # "{\n" on same line as "if" - preparedGrep "namespace .*\{" - preparedGrep "[,\(<]\s*const " # const on left side of type - preparedGrep "^\s*(static)?\s*const " # const on left side of type (beginning of line) - preparedGrep "^ [^*]|[^*] | [^*]" # uses spaces for indentation or mixes spaces and tabs - preparedGrep "[a-zA-Z0-9_]\s*[&][a-zA-Z_]" | grep -E -v "return [&]" # right-aligned reference ampersand (needs to exclude return) - # right-aligned reference pointer star (needs to exclude return and comments) - preparedGrep "[a-zA-Z0-9_]\s*[*][a-zA-Z_]" | grep -E -v -e "return [*]" -e "^* [*]" -e "^*//.*" - # unqualified move()/forward() checks, i.e. make sure that std::move() and std::forward() are used instead of move() and forward() - preparedGrep "move\(.+\)" | grep -v "std::move" | grep -E "[^a-z]move" - preparedGrep "forward\(.+\)" | grep -v "std::forward" | grep -E "[^a-z]forward" -) | grep -E -v -e "^[a-zA-Z\./]*:[0-9]*:\s*\/(\/|\*)" -e "^test/" || true + preparedGrep "#include \"" | grep -E -v -e "license.h" -e "BuildInfo.h" | addPrefix "Use angle brackets for includes" + preparedGrep "\<(if|for|while|switch)\(" | addPrefix "Missing space after keyword" + preparedGrep "\\s*\([^=]*\>\s:\s.*\)" | addPrefix "Missing space before range-based for colon" + preparedGrep "\\s*\(.*\)\s*\{\s*$" | addPrefix "Opening brace on same line as if" + preparedGrep "namespace .*\{" | addPrefix "Missing space before opening brace in namespace" + preparedGrep "[,\(<]\s*const " | addPrefix "const should be on the right side of the type" + preparedGrep "^\s*(static)?\s*const " | addPrefix "const should be on the right side of the type" + preparedGrep "^ [^*]|[^*] | [^*]" | addPrefix "Use tabs for indentation" + preparedGrep "[a-zA-Z0-9_]\s*[&][a-zA-Z_]" | grep -E -v "return [&]" | addPrefix "Reference ampersand should be left-aligned" + preparedGrep "[a-zA-Z0-9_]\s*[*][a-zA-Z_]" | grep -E -v -e "return [*]" -e "^* [*]" -e "^*//.*" | addPrefix "Pointer star should be left-aligned" + preparedGrep "move\(.+\)" | grep -v "std::move" | grep -E "[^a-z]move" | addPrefix "Use std::move" + preparedGrep "forward\(.+\)" | grep -v "std::forward" | grep -E "[^a-z]forward" | addPrefix "Use std::forward" +) | grep -E -v -e "^\[[^]]*\] [a-zA-Z./]*:[0-9]*:\s*/[/*]" -e "^\[[^]]*\] test/" || true ) # Special error handling for `using namespace std;` exclusion, since said statement can be present in the test directory @@ -67,18 +68,21 @@ FORMATERROR=$( # std namespace usage, test directory must also be covered. FORMATSTDERROR=$( ( - git grep -nIE "using namespace std;" -- '*.h' '*.cpp' + git grep -nIE "using namespace std;" -- '*.h' '*.cpp' | addPrefix "Do not use 'using namespace std'" ) || true ) # Merge errors into single string -FORMATEDERRORS="$FORMATERROR$FORMATSTDERROR" +FORMATEDERRORS=$(printf '%s\n' "$FORMATERROR" "$FORMATSTDERROR" | grep -v '^$') if [[ "$FORMATEDERRORS" != "" ]] then echo "Coding style error:" | tee -a "$ERROR_LOG" echo "$FORMATEDERRORS" | tee -a "$ERROR_LOG" - scripts/ci/post_style_errors_on_github.sh "$ERROR_LOG" - exit 1 +fi + +if [[ -s "$ERROR_LOG" ]] +then + exit 1 fi )