From c4c8965513d51fecc76348c9716b55dbb859e0de Mon Sep 17 00:00:00 2001 From: morris Date: Wed, 3 Apr 2024 14:57:54 +0800 Subject: [PATCH] feat(tool): use ast-grep to lint code base --- .gitlab/ci/pre_check.yml | 3 +- components/hal/sdio_slave_hal.c | 8 ++--- sgconfig.yml | 2 ++ tools/ci/check_api_violation.sh | 10 ------ tools/ci/check_examples_rom_header.sh | 21 ----------- tools/ci/exclude_check_tools_files.txt | 1 + tools/ci/executable-list.txt | 1 - .../no_private_rom_api_in_examples.yml | 32 +++++++++++++++++ .../no_std_assert_in_hal_component.yml | 35 +++++++++++++++++++ 9 files changed, 76 insertions(+), 37 deletions(-) create mode 100644 sgconfig.yml delete mode 100755 tools/ci/check_examples_rom_header.sh create mode 100644 tools/ci/sg_rules/no_private_rom_api_in_examples.yml create mode 100644 tools/ci/sg_rules/no_std_assert_in_hal_component.yml diff --git a/.gitlab/ci/pre_check.yml b/.gitlab/ci/pre_check.yml index 6c104e65c7..8f50f30ca3 100644 --- a/.gitlab/ci/pre_check.yml +++ b/.gitlab/ci/pre_check.yml @@ -25,7 +25,8 @@ check_version: check_api_usage: extends: .pre_check_template script: - - tools/ci/check_examples_rom_header.sh + - python -m pip install ast-grep-cli # use ast-grep to describe customized lint rules + - ast-grep scan - tools/ci/check_api_violation.sh - tools/ci/check_examples_extra_component_dirs.sh diff --git a/components/hal/sdio_slave_hal.c b/components/hal/sdio_slave_hal.c index be81ca8b15..21f4c4c158 100644 --- a/components/hal/sdio_slave_hal.c +++ b/components/hal/sdio_slave_hal.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -190,7 +190,7 @@ static esp_err_t init_send_queue(sdio_slave_context_t *hal) //loop in the ringbuf to link all the desc one after another as a ring for (int i = 0; i < hal->send_queue_size + 1; i++) { rcv_res = sdio_ringbuf_recv(buf, &last, NULL, RINGBUF_GET_ONE); - assert (rcv_res == ESP_OK); + HAL_ASSERT(rcv_res == ESP_OK); ret = sdio_ringbuf_send(buf, link_desc_to_last, last); if (ret != ESP_OK) return ret; @@ -202,7 +202,7 @@ static esp_err_t init_send_queue(sdio_slave_context_t *hal) last = NULL; //clear the queue rcv_res = sdio_ringbuf_recv(buf, &first, &last, RINGBUF_GET_ALL); - assert (rcv_res == ESP_OK); + HAL_ASSERT(rcv_res == ESP_OK); HAL_ASSERT(first == last); //there should be only one desc remain sdio_ringbuf_return(buf, (uint8_t *) first); return ESP_OK; @@ -634,7 +634,7 @@ void sdio_slave_hal_recv_flush_one_buffer(sdio_slave_context_t *hal) { sdio_slave_hal_recv_stailq_t *const queue = &hal->recv_link_list; sdio_slave_ll_desc_t *desc = STAILQ_FIRST(queue); - assert (desc != NULL && desc->owner == 0); + HAL_ASSERT(desc != NULL && desc->owner == 0); STAILQ_REMOVE_HEAD(queue, qe); desc->owner = 1; STAILQ_INSERT_TAIL(queue, desc, qe); diff --git a/sgconfig.yml b/sgconfig.yml new file mode 100644 index 0000000000..8f89c92acf --- /dev/null +++ b/sgconfig.yml @@ -0,0 +1,2 @@ +ruleDirs: +- ./tools/ci/sg_rules diff --git a/tools/ci/check_api_violation.sh b/tools/ci/check_api_violation.sh index 7cf673fdbd..ce394681a3 100755 --- a/tools/ci/check_api_violation.sh +++ b/tools/ci/check_api_violation.sh @@ -20,13 +20,3 @@ if [ $count -gt 0 ]; then echo "Please try to use the APIs listed in esp_rom/include/esp_rom_xxx.h" exit 1 fi - -# ESP-IDF `hal` component shouldn't call "assert()" directlly -files_to_search=$(git ls-files --full-name 'components/hal/*' | grep -v components/hal/test_apps/) -found_libc_assert=$(grep -E '\W+assert\(' $files_to_search) -if [ -n "$found_libc_assert" ]; then - echo "hal assert violation" - echo $found_libc_assert - echo "Please use HAL_ASSERT() instead of assert() in hal component" - exit 1 -fi diff --git a/tools/ci/check_examples_rom_header.sh b/tools/ci/check_examples_rom_header.sh deleted file mode 100755 index 5a1c9aa633..0000000000 --- a/tools/ci/check_examples_rom_header.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/usr/bin/env bash - -set -uo pipefail - -# Examples shouldn't include rom headers directly - -output=$(find ${IDF_PATH}/examples -name "*.[chS]" -o -name "*.cpp" -not -path "**/build/**") -files=$(egrep ".*include.*\Wrom\W.*h" ${output} | cut -d ":" -f 1) -found_rom=0 -for file in ${files} -do - echo "${file} contains rom headers!" - ((found_rom++)) -done - -if [ $found_rom -eq 0 ]; then - echo "No rom headers found in examples" - exit 0 -fi - -exit 1 diff --git a/tools/ci/exclude_check_tools_files.txt b/tools/ci/exclude_check_tools_files.txt index 8fa184a3c3..4839470759 100644 --- a/tools/ci/exclude_check_tools_files.txt +++ b/tools/ci/exclude_check_tools_files.txt @@ -49,3 +49,4 @@ tools/ci/python_packages/idf_http_server_test/**/* tools/ci/python_packages/idf_iperf_test_util/**/* tools/esp_prov/**/* tools/ci/sort_yaml.py +tools/ci/sg_rules/* diff --git a/tools/ci/executable-list.txt b/tools/ci/executable-list.txt index 187a49239e..bc96fcf460 100644 --- a/tools/ci/executable-list.txt +++ b/tools/ci/executable-list.txt @@ -55,7 +55,6 @@ tools/ci/check_codeowners.py tools/ci/check_deprecated_kconfigs.py tools/ci/check_esp_memory_utils_headers.sh tools/ci/check_examples_extra_component_dirs.sh -tools/ci/check_examples_rom_header.sh tools/ci/check_executables.py tools/ci/check_idf_version.sh tools/ci/check_kconfigs.py diff --git a/tools/ci/sg_rules/no_private_rom_api_in_examples.yml b/tools/ci/sg_rules/no_private_rom_api_in_examples.yml new file mode 100644 index 0000000000..ee8dc3d7c2 --- /dev/null +++ b/tools/ci/sg_rules/no_private_rom_api_in_examples.yml @@ -0,0 +1,32 @@ +# Refer to https://ast-grep.github.io/guide/rule-config.html for Rule Essentials +id: no-private-rom-api-in-c-examples +message: Don't use private ROM APIs in the examples +severity: error # error, warning, info, hint +note: Only APIs prefixed with "esp_rom_" are treated as public. +language: C +files: + - "./examples/**/*" +rule: + kind: preproc_include + has: + field: path + regex: "rom/.*h" + pattern: $N +fix: '' + +--- + +id: no-private-rom-api-in-cpp-examples +message: Don't use private ROM APIs in the examples +severity: error # error, warning, info, hint +note: Only APIs prefixed with "esp_rom_" are treated as public. +language: Cpp +files: + - "./examples/**/*" +rule: + kind: preproc_include + has: + field: path + regex: "rom/.*h" + pattern: $N +fix: '' diff --git a/tools/ci/sg_rules/no_std_assert_in_hal_component.yml b/tools/ci/sg_rules/no_std_assert_in_hal_component.yml new file mode 100644 index 0000000000..5f6c3efdba --- /dev/null +++ b/tools/ci/sg_rules/no_std_assert_in_hal_component.yml @@ -0,0 +1,35 @@ +# Refer to https://ast-grep.github.io/guide/rule-config.html for Rule Essentials +id: no-std-assert-call-in-hal-component +message: Don't use standard assert function in the hal component +severity: error # error, warning, info, hint +note: The standard assert function depends on newlib(G1) component, but hal is a G0 component +language: C +files: + - "./components/hal/**/*" +ignores: + - "./components/hal/test_apps/**/*" +rule: + kind: expression_statement + pattern: assert($$$ARGS); +fix: HAL_ASSERT($$$ARGS); + +--- + +id: no-std-assert-include-in-hal-component +message: Don't include assert.h in the hal component +severity: error # error, warning, info, hint +note: Please use hal/assert.h to replace assert.h +language: C +files: + - "./components/hal/**/*" +ignores: + - "./components/hal/test_apps/**/*" +rule: + kind: preproc_include + has: + field: path + pattern: $N +constraints: + N: + regex: '^["<]assert' # match "assert.h" or +fix: ''