-
Notifications
You must be signed in to change notification settings - Fork 97
feat: CI - Add code rules checking action #3914
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
6f8f1c0
243af07
7754898
29f0ed6
3d2782a
38748b0
21956b3
183262c
1a50cc9
4004d12
42d6f5b
6a87699
3ae12c4
06b6d8f
9f65bda
e519b29
09cea71
716ce69
a7f4a14
fad5944
d3a6d9c
56eebfc
933dcc0
fc78670
3716bf2
d2eddbd
e59d2f0
5c43982
883ba54
02e26e1
a24ff21
e28b1c2
27657f6
f48fc29
cb5754e
4aeafa3
e9e58c2
0792e88
14d9399
b7a5e53
7ed6c89
1b6654e
a4d11fd
198e895
a95fba7
239fb10
87d05fe
010d4bc
5f6c1ad
35a5aa4
64479d0
be824c6
0845150
2a96409
1d0f7f7
a47e596
411913e
d10651d
ae7e57c
b30ef2a
0dbd595
7f5aa44
84d15e9
bc8e010
74cad67
44f564e
46a0cf0
addf93f
cf5fea6
3444008
72e8258
10d7a2e
2ebbc31
a1b5b2f
92a66f3
e207a68
d2b7019
f73cb6d
c8d97a4
1632aee
679db25
90f9dba
39389fd
eb30bc1
048aa63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,6 +137,22 @@ jobs: | |
| RUNS_ON: ubuntu-22.04 | ||
| USE_SCCACHE: false | ||
|
|
||
| check_code_rules: | ||
| needs: [is_not_draft_pull_request] | ||
| runs-on: ubuntu-22.04 | ||
| steps: | ||
| - name: Checkout Repository | ||
| uses: actions/[email protected] | ||
| with: | ||
| submodules: false | ||
| lfs: false | ||
| fetch-depth: 0 | ||
| sparse-checkout: | | ||
| scripts | ||
| src | ||
| - name: Check the code rules | ||
| run: "scripts/check_code_rules.sh" | ||
|
|
||
| # Matrix containing all the CPU build. | ||
| # Those are quite fast and can efficiently benefit from the `sccache' tool to make them even faster. | ||
| cpu_builds: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,151 @@ | ||||||||||
| #!/bin/bash | ||||||||||
|
|
||||||||||
|
|
||||||||||
| ################################ | ||||||||||
| ## I. Function initializations | ||||||||||
| ################################ | ||||||||||
|
|
||||||||||
| # while | ||||||||||
| # ... done < <(grep -n "std::map\s*<" "$file") | ||||||||||
| # Process the result of grep command as a file. | ||||||||||
| # This allows everything to be handled in the same shell environment. | ||||||||||
| check_container_usage() { | ||||||||||
| local container_name="$1" | ||||||||||
| local str=" Found forbidden ${container_name} usage in: $file"$'\n' | ||||||||||
|
|
||||||||||
| if grep -q "${container_name}\s*<" "$file"; then | ||||||||||
| while IFS= read -r line; do | ||||||||||
| str+=" $line"$'\n' | ||||||||||
| done < <(grep -n "${container_name}\s*<" "$file") | ||||||||||
|
|
||||||||||
| ERRORS_CONTAINER[$container_name]+="$str" | ||||||||||
| ((FORBIDDEN_CONTAINER_MAP["$container_name"]++)) | ||||||||||
| fi | ||||||||||
| } | ||||||||||
|
|
||||||||||
| print_violation() | ||||||||||
| { | ||||||||||
| local container_name="$1" | ||||||||||
|
|
||||||||||
| if [ "${FORBIDDEN_CONTAINER_MAP[$container_name]}" -eq 1 ];then | ||||||||||
| echo $'\n' | ||||||||||
| echo "ERROR: Forbidden $container_name usage detected" | ||||||||||
| echo "==========================================" | ||||||||||
|
|
||||||||||
| printf '%s' "${ERRORS_CONTAINER[$container_name]}" | ||||||||||
|
|
||||||||||
| fi | ||||||||||
| } | ||||||||||
|
|
||||||||||
| ################################ | ||||||||||
| ## II. GLOBAL INITIALIZATION | ||||||||||
| ################################ | ||||||||||
|
|
||||||||||
| FORBIDDEN_EXPRESSIONS=( | ||||||||||
| "std::map" | ||||||||||
| "std::unordered_map" | ||||||||||
| "std::vector" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| declare -A FORBIDDEN_CONTAINER_MAP=() | ||||||||||
| for exp in "${FORBIDDEN_EXPRESSIONS[@]}"; do | ||||||||||
| FORBIDDEN_CONTAINER_MAP["$exp"]=0 | ||||||||||
| done | ||||||||||
|
|
||||||||||
| declare -A ERRORS_CONTAINER=() | ||||||||||
| for exp in "${FORBIDDEN_EXPRESSIONS[@]}"; do | ||||||||||
| ERRORS_CONTAINER["$exp"]="" | ||||||||||
| done | ||||||||||
|
|
||||||||||
| FILE_PREFIX="src/coreComponents/" | ||||||||||
| FILE_PATTERNS=( | ||||||||||
| "codingUtilities" | ||||||||||
| "common" | ||||||||||
| "dataRepository" | ||||||||||
| "constitutive" | ||||||||||
| "denseLinearAlgebra" | ||||||||||
| "discretizationMethods" | ||||||||||
| "events" | ||||||||||
| "fieldSpecification" | ||||||||||
| "fileIO" | ||||||||||
| "finiteElement" | ||||||||||
| "finiteVolume" | ||||||||||
| "functions" | ||||||||||
| "linearAlgebra" | ||||||||||
| "mainInterface" | ||||||||||
| "mesh" | ||||||||||
| "physicsSolvers" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| EXCLUDE_PATTERNS=( | ||||||||||
| "Datatype.hpp" | ||||||||||
| "StdContainerWrappers.hpp" | ||||||||||
| "BufferOps_inline.hpp" | ||||||||||
| "BufferOps.hpp" | ||||||||||
| "PVTPackage" | ||||||||||
| "hdf5_interface" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| FILE_PATH_ARGS=() | ||||||||||
| for pattern in "${FILE_PATTERNS[@]}"; do | ||||||||||
| if [ -d "${FILE_PREFIX}${pattern}" ]; then | ||||||||||
| FILE_PATH_ARGS+=("${FILE_PREFIX}${pattern}") | ||||||||||
| fi | ||||||||||
| done | ||||||||||
|
|
||||||||||
| EXCLUDE_EXPRESSION=() | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused, does it excludes?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I make it clearer
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Suggested change
|
||||||||||
| for pattern in "${EXCLUDE_PATTERNS[@]}"; do | ||||||||||
| if [[ ! "$pattern" == *".hpp"* ]]; then | ||||||||||
| EXCLUDE_EXPRESSION+=( -path "*/$pattern" -prune -o ) | ||||||||||
| else | ||||||||||
| EXCLUDE_EXPRESSION+=( -name "*${pattern}" -prune -o ) | ||||||||||
| fi | ||||||||||
| done | ||||||||||
|
|
||||||||||
| echo "======================================================" | ||||||||||
| echo -e " TREE LIST OF FILES FOUND " | ||||||||||
| echo "======================================================" | ||||||||||
|
|
||||||||||
| if [ ${#FILE_PATH_ARGS[@]} -gt 0 ]; then | ||||||||||
| find "${FILE_PATH_ARGS[@]}" "${EXCLUDE_EXPRESSION[@]}" -type d -print | sort | while IFS= read -r dir; do | ||||||||||
| echo -e "->" $(echo "$dir" | sed 's/[]^$*.[]/\\&/g' | sed 's/\// |---/g') | ||||||||||
| done | ||||||||||
| fi | ||||||||||
|
|
||||||||||
| ARRAY_FILES=() | ||||||||||
| # mapfile used for reading inMAPput lines into an array; -d $'\0': Specifies that the delimiter is (\0). | ||||||||||
| # -print0 : ask find to separate file paths by '\0' | ||||||||||
| mapfile -d $'\0' ARRAY_FILES < <(find "${FILE_PATH_ARGS[@]}" "${EXCLUDE_EXPRESSION[@]}" \ | ||||||||||
| -type f \( -name "*.hpp" -o -name "*.cpp" -o -name "*.hpp.template" -o -name "*.cpp.template" \) \ | ||||||||||
| -print0 2>/dev/null) | ||||||||||
|
|
||||||||||
| ################################ | ||||||||||
| ## III. MAIN LOOP | ||||||||||
| ################################ | ||||||||||
|
|
||||||||||
| for file in "${ARRAY_FILES[@]}"; do | ||||||||||
| for container_name in "${!FORBIDDEN_CONTAINER_MAP[@]}"; do | ||||||||||
| check_container_usage "$container_name" | ||||||||||
| done | ||||||||||
| done | ||||||||||
|
|
||||||||||
| # Print section | ||||||||||
| for key in "${!FORBIDDEN_CONTAINER_MAP[@]}"; do | ||||||||||
| if [[ "${FORBIDDEN_CONTAINER_MAP[$key]}" == "1" ]]; then | ||||||||||
| echo $'\n' | ||||||||||
| echo "----------------------------------------" | ||||||||||
| echo "SUMMARY: Code rule violations found" | ||||||||||
| echo "----------------------------------------" | ||||||||||
|
|
||||||||||
| for container_name in "${!FORBIDDEN_CONTAINER_MAP[@]}"; do | ||||||||||
| print_violation "$container_name" | ||||||||||
| done | ||||||||||
|
|
||||||||||
| echo "" | ||||||||||
| exit 1; | ||||||||||
| fi | ||||||||||
| done | ||||||||||
|
|
||||||||||
| echo $'\n' | ||||||||||
| echo "No code rule violations found" | ||||||||||
| exit 0 | ||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it no longer grouped with the code style & docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not executed the same way as code style & docs
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, why isn't it executed the same way? I don't know why the process has to be different than uncrustify check.
And, if it have to remain a job, why isn't
check_code_rulesincheck_that_all_jobs_succeeded? As you're proposing, it is only advisory.