Skip to content

Commit 5fe294d

Browse files
zrr1999SigureMo
zrr1999
and
SigureMo
authored
[CodeStyle] Integrate ast-grep to pre-commit hooks (#76313) (#76339)
Co-authored-by: Nyakku Shigure <[email protected]>
1 parent 14509d0 commit 5fe294d

13 files changed

+179
-22
lines changed

‎.github/workflows/Codestyle-Check.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ jobs:
4949
if: steps.check-bypass.outputs.can-skip != 'true'
5050
run: |
5151
pip install pre-commit==2.17.0 cpplint==1.6.0 clang-format==13.0.0
52+
pip install ast-grep-cli==0.39.9 # This version should be consistent with the one in .pre-commit-config.yaml
53+
54+
- name: Run ast-grep unit tests
55+
if: steps.check-bypass.outputs.can-skip != 'true'
56+
run: |
57+
ast-grep test
5258
5359
- name: Check pre-commit
5460
if: steps.check-bypass.outputs.can-skip != 'true'

‎.pre-commit-config.yaml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ repos:
3737
(?x)^(
3838
test/dygraph_to_static/test_error.py
3939
)$
40+
- repo: https://github.com/PFCCLab/ast-grep-pre-commit-mirror
41+
rev: v0.39.9
42+
hooks:
43+
- id: ast-grep
4044
- repo: local
4145
hooks:
4246
- id: copyright_checker
@@ -158,7 +162,10 @@ repos:
158162
(?x)^(
159163
\.github/.+\.(yaml|yml)|
160164
\.pre-commit-config\.yaml|
161-
\.yamlfmt
165+
\.yamlfmt|
166+
sgconfig\.yml|
167+
ci/rules/.+\.yml|
168+
ci/rule-tests/.+\.yml
162169
)
163170
# Others
164171
- repo: local

‎ci/check_approval.sh‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -330,19 +330,6 @@ if [ "${EMPTY_GRAD_OP_REGISTERED}" != "" ] && [ "${GIT_PT_ID}" != "" ]; then
330330
check_approval 1 phlrain XiaoguangHu01 kolinwei JiabinYang
331331
fi
332332

333-
INVALID_UNITTEST_ASSERT_CHECK=`echo "$ALL_ADDED_LINES" | grep -zoE '\+\s+((assert\s+)|(self\.assert(True|Equal)\())(\s*\+\s*)?(np|numpy)\.(allclose|array_equal)[^+]*' || true`
334-
if [ "${INVALID_UNITTEST_ASSERT_CHECK}" != "" ] && [ "${PR_ID}" != "" ]; then
335-
echo_line="It is recommended to use 'np.testing.assert_allclose' and 'np.testing.assert_array_equal' instead of 'self.assertTrue(np.allclose(...))' and 'self.assertTrue(np.array_equal(...))'.\nPlease modify the code below. If anything is unclear, please read the specification [ https://github.com/PaddlePaddle/community/blob/master/rfcs/CodeStyle/20220805_code_style_improvement_for_unittest.md#background ]. If it is a mismatch, please request SigureMo (Recommend) or zrr1999 or luotao1 review and approve.\nThe code that do not meet the specification are as follows:\n${INVALID_UNITTEST_ASSERT_CHECK}\n"
336-
check_approval 1 SigureMo zrr1999 luotao1
337-
fi
338-
339-
TEST_FILE_ADDED_LINES=$(git diff -U0 upstream/$BRANCH -- test |grep "^+")
340-
ENABLE_TO_STATIC_CHECK=`echo "$TEST_FILE_ADDED_LINES" | grep "enable_to_static(" || true`
341-
if [ "${ENABLE_TO_STATIC_CHECK}" != "" ] && [ "${PR_ID}" != "" ]; then
342-
echo_line="You must have one RD (SigureMo, DrRyanHuang, zrr1999 or gouzil) approval for using 'paddle.jit.enable_to_static', we recommend using 'enable_to_static_guard' in the related test files.\n"
343-
check_approval 1 SigureMo DrRyanHuang zrr1999 gouzil
344-
fi
345-
346333
HAS_MODIFIED_DY2ST_TEST_FILES=$(git diff --name-only --diff-filter=ACMR upstream/$BRANCH | grep "test/dygraph_to_static/test_" || true)
347334
if [ "${HAS_MODIFIED_DY2ST_TEST_FILES}" != "" ] && [ "${PR_ID}" != "" ]; then
348335
error_lines=`python ${PADDLE_ROOT}/test/dygraph_to_static/check_approval.py ${HAS_MODIFIED_DY2ST_TEST_FILES}`

‎ci/rule-tests/__snapshots__/no-assert-with-numpy-testing-snapshot.yml‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
id: no-assert-with-numpy-testing
2+
snapshots:
3+
assert np.allclose(a, b, rtol=1e-5):
4+
labels:
5+
- source: assert np.allclose(a, b, rtol=1e-5)
6+
style: primary
7+
start: 0
8+
end: 35
9+
assert np.array_equal(a, b, err_msg="Arrays are not equal"):
10+
labels:
11+
- source: assert np.array_equal(a, b, err_msg="Arrays are not equal")
12+
style: primary
13+
start: 0
14+
end: 59
15+
self.assertEqual(np.allclose(a, b), something):
16+
labels:
17+
- source: self.assertEqual(np.allclose(a, b), something)
18+
style: primary
19+
start: 0
20+
end: 46
21+
self.assertEqual(np.array_equal(a, b), True):
22+
labels:
23+
- source: self.assertEqual(np.array_equal(a, b), True)
24+
style: primary
25+
start: 0
26+
end: 44
27+
self.assertEqual(np.array_equal(a, b), True, err_msg="Arrays are not equal"):
28+
labels:
29+
- source: self.assertEqual(np.array_equal(a, b), True, err_msg="Arrays are not equal")
30+
style: primary
31+
start: 0
32+
end: 76
33+
self.assertEqual(something, np.allclose(a, b)):
34+
labels:
35+
- source: self.assertEqual(something, np.allclose(a, b))
36+
style: primary
37+
start: 0
38+
end: 46
39+
self.assertTrue(np.allclose(a, b)):
40+
labels:
41+
- source: self.assertTrue(np.allclose(a, b))
42+
style: primary
43+
start: 0
44+
end: 34
45+
self.assertTrue(np.allclose(a, b, rtol=1e-5, atol=1e-8)):
46+
labels:
47+
- source: self.assertTrue(np.allclose(a, b, rtol=1e-5, atol=1e-8))
48+
style: primary
49+
start: 0
50+
end: 56
51+
self.assertTrue(np.array_equal(a, b)):
52+
labels:
53+
- source: self.assertTrue(np.array_equal(a, b))
54+
style: primary
55+
start: 0
56+
end: 37
57+
self.assertTrue(np.array_equal(a, b), "Arrays are not equal"):
58+
labels:
59+
- source: self.assertTrue(np.array_equal(a, b), "Arrays are not equal")
60+
style: primary
61+
start: 0
62+
end: 61

‎ci/rule-tests/__snapshots__/no-enable-to-static-snapshot.yml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
id: no-enable-to-static
2+
snapshots:
3+
enable_to_static(True):
4+
labels:
5+
- source: enable_to_static(True)
6+
style: primary
7+
start: 0
8+
end: 22
9+
jit.enable_to_static(False):
10+
labels:
11+
- source: jit.enable_to_static(False)
12+
style: primary
13+
start: 0
14+
end: 27
15+
paddle.jit.enable_to_static(True):
16+
labels:
17+
- source: paddle.jit.enable_to_static(True)
18+
style: primary
19+
start: 0
20+
end: 33

‎ci/rule-tests/no-assert-with-numpy-testing-test.yml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
id: no-assert-with-numpy-testing
2+
valid:
3+
- np.testing.assert_allclose(a, b)
4+
- np.testing.assert_array_equal(a, b)
5+
- np.testing.assert_allclose(a, b, rtol=1e-5, atol=1e-8)
6+
- np.testing.assert_array_equal(a, b, err_msg="Arrays are not equal")
7+
- assert paddle.allclose(a, b, rtol=1e-5)
8+
- assert torch.array_equal(a, b)
9+
- self.assertTrue(numpy)
10+
invalid:
11+
- self.assertTrue(np.allclose(a, b))
12+
- self.assertTrue(np.allclose(a, b, rtol=1e-5, atol=1e-8))
13+
- self.assertTrue(np.array_equal(a, b))
14+
- self.assertTrue(np.array_equal(a, b), "Arrays are not equal")
15+
- self.assertEqual(something, np.allclose(a, b))
16+
- self.assertEqual(np.allclose(a, b), something)
17+
- self.assertEqual(np.array_equal(a, b), True)
18+
- self.assertEqual(np.array_equal(a, b), True, err_msg="Arrays are not equal")
19+
- assert np.allclose(a, b, rtol=1e-5)
20+
- assert np.array_equal(a, b, err_msg="Arrays are not equal")

‎ci/rule-tests/no-enable-to-static-test.yml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
id: no-enable-to-static
2+
valid:
3+
- |
4+
with enable_to_static_guard(True):
5+
# test code here
6+
- enable_to_static_guard(False)
7+
invalid:
8+
- paddle.jit.enable_to_static(True)
9+
- jit.enable_to_static(False)
10+
- enable_to_static(True)

‎ci/rules/no-assert-with-numpy-testing.yml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
id: no-assert-with-numpy-testing
2+
language: python
3+
severity: error
4+
message: |
5+
It is recommended to use 'np.testing.assert_allclose' and 'np.testing.assert_array_equal'
6+
instead of 'self.assertTrue(np.allclose(...))' and 'self.assertTrue(np.array_equal(...))'.
7+
Please read the specification: https://github.com/PaddlePaddle/community/blob/master/rfcs/CodeStyle/20220805_code_style_improvement_for_unittest.md#background
8+
If it is a false positive, please contact SigureMo (Recommend) or zrr1999 for more information.
9+
note: Use np.testing methods for more informative assertion messages
10+
files:
11+
- test/**
12+
13+
rule:
14+
any:
15+
- pattern: self.assertTrue($NP.$CHECK_EQ($$$ARGS) $$$_)
16+
- pattern: self.assertEqual($ARG1, $NP.$CHECK_EQ($$$ARGS) $$$_)
17+
- pattern: self.assertEqual($NP.$CHECK_EQ($$$ARGS), $ARG2 $$$_)
18+
- pattern: assert $NP.$CHECK_EQ($$$ARGS)
19+
constraints:
20+
NP:
21+
regex: (np|numpy)
22+
CHECK_EQ:
23+
regex: (allclose|array_equal)

‎ci/rules/no-enable-to-static.yml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
id: no-enable-to-static
2+
language: python
3+
severity: error
4+
message: |
5+
Use `paddle.jit.enable_to_static` will caused some test run in dygraph mode. Recommended
6+
to use `enable_to_static_guard` to avoid this problem.
7+
If it is a false positive, please contact SigureMo (Recommend), DrRyanHuang or zrr1999 for more information.
8+
note: Use `enable_to_static_guard` to replace `paddle.jit.enable_to_static` in test files
9+
files:
10+
- test/**
11+
12+
rule:
13+
any:
14+
- pattern: $$$PREFIX.enable_to_static($$$ARGS)
15+
- pattern: enable_to_static($$$ARGS)
16+
constraints:
17+
PREFIX:
18+
regex: (paddle\.jit|jit)

‎sgconfig.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
ruleDirs:
2+
- ci/rules
3+
testConfigs:
4+
- testDir: ci/rule-tests

0 commit comments

Comments
 (0)