Skip to content

Commit 60c4c59

Browse files
gasparevgasparev
authored
Merge pull request #27 from Qarik-Group/gasparev/manage-duplicate-section
Manage duplicated sections
2 parents 75829ef + b3ff8de commit 60c4c59

10 files changed

Lines changed: 119 additions & 29 deletions

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ gitlab-codeowners-linter makes sure that the CODEOWNERS file is formatted respec
88
- paths in a section must be unique
99
- there must be no empty lines between paths
1010
- paths must exist
11+
- there must not be duplicated sections
1112

1213
The linter can run in check or autofix mode.
1314

‎gitlab_codeowners_linter/autofix.py‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
from __future__ import annotations
22

3-
import operator
43
from functools import cmp_to_key
54

65
from gitlab_codeowners_linter.constants import DEFAULT_SECTION
76
from gitlab_codeowners_linter.sorting import sort_paths
7+
from gitlab_codeowners_linter.sorting import sort_section_names
88

99

1010
def fix(codeowners_data, violations, file_path):
@@ -13,15 +13,34 @@ def fix(codeowners_data, violations, file_path):
1313

1414
# Are custom section names sorted?
1515
if violations.section_names_sorted:
16+
sort_sections_names_key = cmp_to_key(sort_section_names)
1617
sorted_sections_names = sorted(
1718
codeowners_data[1:],
18-
key=operator.attrgetter('codeowner_section'),
19+
key=sort_sections_names_key,
1920
)
2021
codeowners_data_updated = []
2122
codeowners_data_updated.append(codeowners_data[0])
2223
codeowners_data_updated.extend(sorted_sections_names)
2324
codeowners_data = codeowners_data_updated
2425

26+
# Are there duplicated sections?
27+
28+
if violations.duplicated_sections != []:
29+
# If multiple sections have the same name, they are combined.
30+
# Also, section headings are not case-sensitive.
31+
# For example the entries defined under the sections Documentation
32+
# and DOCUMENTATION are combined, using the case of the first section
33+
i = 0
34+
while i < len(codeowners_data)-1:
35+
if codeowners_data[i].codeowner_section.lower() == codeowners_data[i+1].codeowner_section.lower():
36+
codeowners_data[i].comments = codeowners_data[i].comments + \
37+
codeowners_data[i+1].comments
38+
codeowners_data[i].entries = codeowners_data[i].entries + \
39+
codeowners_data[i+1].entries
40+
codeowners_data.pop(i+1)
41+
else:
42+
i += 1
43+
2544
# Then fix section's content
2645

2746
codeowners_data_updated = []
@@ -34,14 +53,14 @@ def fix(codeowners_data, violations, file_path):
3453
else:
3554
codeowners_data_updated[-1] = _fix_blank_lines(
3655
codeowners_data_updated[-1])
37-
if violations.unsorted_paths_in_sections != []:
38-
if not section.codeowner_section in violations.unsorted_paths_in_sections:
56+
if violations.unsorted_paths_in_sections != [] or violations.duplicated_sections != []:
57+
if not section.codeowner_section in violations.unsorted_paths_in_sections and violations.duplicated_sections == []:
3958
pass
4059
else:
4160
codeowners_data_updated[-1] = _fix_unsorted_paths(
4261
codeowners_data_updated[-1])
43-
if violations.sections_with_duplicate_paths != []:
44-
if not section.codeowner_section in violations.sections_with_duplicate_paths:
62+
if violations.sections_with_duplicate_paths != [] or violations.duplicated_sections != []:
63+
if not section.codeowner_section in violations.sections_with_duplicate_paths and violations.duplicated_sections == []:
4564
pass
4665
else:
4766
codeowners_data_updated[-1] = _fix_duplicated_paths(
@@ -51,7 +70,7 @@ def fix(codeowners_data, violations, file_path):
5170
pass
5271
else:
5372
codeowners_data_updated[-1] = _fix_nonexisting_paths(
54-
codeowners_data_updated[-1], violations.non_existing_paths[violations.sections_with_non_existing_paths.index(section.codeowner_section)])
73+
codeowners_data_updated[-1], violations.non_existing_paths[section.codeowner_section.lower()])
5574
codeowners_data = codeowners_data_updated
5675

5776
_update_codeowners_file(codeowners_data, file_path)

‎gitlab_codeowners_linter/checks.py‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
from pathspec import PathSpec
77

88
from gitlab_codeowners_linter.sorting import sort_paths
9+
from gitlab_codeowners_linter.sorting import sort_section_names
10+
11+
# TODO: if there are duplicated sections, do I want to issue the error message for
12+
# that section only once?
913

1014

1115
class CodeownersViolations:
@@ -16,7 +20,8 @@ def __init__(self):
1620
self.unsorted_paths_in_sections = []
1721
self.sections_with_duplicate_paths = []
1822
self.sections_with_non_existing_paths = []
19-
self.non_existing_paths = []
23+
self.non_existing_paths = {}
24+
self.duplicated_sections = []
2025

2126

2227
def check(codeowners_data):
@@ -26,14 +31,18 @@ def check(codeowners_data):
2631
return violations
2732

2833
# Are custom section names sorted?
29-
if [
30-
section.codeowner_section for section in codeowners_data[1:]
31-
] != sorted(
32-
section.codeowner_section for section in codeowners_data[1:]
33-
):
34+
sort_sections_names_key = cmp_to_key(sort_section_names)
35+
if codeowners_data[1:] != sorted(codeowners_data[1:], key=sort_sections_names_key):
3436
violations.violation_error_messages.append('Sections are not sorted')
3537
violations.section_names_sorted = True
3638

39+
# Are there duplicated sections?
40+
violations.duplicated_sections = _get_duplicated_sections(codeowners_data)
41+
if violations.duplicated_sections != []:
42+
violations.violation_error_messages.append(
43+
f"The sections {', '.join(map(str, violations.duplicated_sections))} are duplicates",
44+
)
45+
3746
# Are there blank lines in sections?
3847
violations.sections_with_blank_lines = _get_sections_with_blank_lines(
3948
codeowners_data)
@@ -76,6 +85,13 @@ def _is_codeowners_empty(codeowners_data):
7685
return empty
7786

7887

88+
def _get_duplicated_sections(codeowners_data):
89+
all_sections_name = list(
90+
section.codeowner_section for section in codeowners_data)
91+
seen = set()
92+
return [x for x in all_sections_name if x.lower() in seen or seen.add(x.lower())]
93+
94+
7995
def _get_sections_with_blank_lines(codeowners_data):
8096
sections_with_blank_lines = []
8197
for section in codeowners_data:
@@ -120,7 +136,7 @@ def _get_all_filepaths():
120136

121137
def _get_non_existing_paths(codeowners_data):
122138
sections_with_non_existing_paths = []
123-
non_existing_paths = []
139+
non_existing_paths = {}
124140
files = _get_all_filepaths()
125141
for section in codeowners_data:
126142
spec = PathSpec.from_lines(
@@ -134,6 +150,11 @@ def _get_non_existing_paths(codeowners_data):
134150
if non_existing_paths_in_section:
135151
sections_with_non_existing_paths.append(
136152
section.codeowner_section)
137-
non_existing_paths.append(non_existing_paths_in_section)
153+
if section.codeowner_section.lower() in non_existing_paths.keys():
154+
non_existing_paths[section.codeowner_section.lower()].extend(
155+
non_existing_paths_in_section)
156+
else:
157+
non_existing_paths[section.codeowner_section.lower(
158+
)] = non_existing_paths_in_section
138159

139160
return sections_with_non_existing_paths, non_existing_paths

‎gitlab_codeowners_linter/codeowners_linter.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
# - there must be no empty lines between paths
77
# - paths in a section must be unique
88
# - paths must exist
9+
# - there must be no duplicated sections
910
#
1011
from __future__ import annotations
1112

‎gitlab_codeowners_linter/sorting.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,11 @@ def sort_paths(entry1, entry2):
2020
return -1
2121

2222
return -1 if (line1 < line2) else 1
23+
24+
25+
def sort_section_names(section1, section2):
26+
section_name1 = section1.codeowner_section.lower()
27+
section_name2 = section2.codeowner_section.lower()
28+
if section_name1 == section_name2:
29+
return 0
30+
return -1 if (section_name1 < section_name2) else 1

‎tests/codeowners_linter_tests.py‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,11 @@ class TestCase:
188188
'resources/existing_paths_input.txt',
189189
),
190190
expected_check=[
191-
'The paths in sections __default_codeowner_section__, SECURITY, SYSTEM are not sorted',
191+
'Sections are not sorted',
192+
'The sections SECURITY, security are duplicates',
193+
'The paths in sections __default_codeowner_section__, Security, SYSTEM, SECURITY are not sorted',
192194
'The sections __default_codeowner_section__ have duplicate paths',
193-
'The sections __default_codeowner_section__, SECURITY, SYSTEM have non-existing paths'],
195+
'The sections __default_codeowner_section__, Security, SYSTEM, SECURITY, security have non-existing paths'],
194196
expected_fix=os.path.join(
195197
os.path.dirname(os.path.abspath(__file__)),
196198
'resources/existing_paths_autofix.txt',
@@ -283,9 +285,10 @@ class TestCase:
283285
),
284286
expected_check=[
285287
'Sections are not sorted',
286-
'There are blank lines in the sections SECTION_NAME, BUILD, SECURITY',
287-
'The paths in sections SECTION_NAME, BUILD, SYSTEM, TEST_SECTION are not sorted',
288-
'The sections SECTION_NAME have duplicate paths',
288+
'The sections SECTION_NAME, section_name, Section_Name are duplicates',
289+
'There are blank lines in the sections Section_name, BUILD, SECURITY',
290+
'The paths in sections Section_name, BUILD, SYSTEM, SECTION_NAME, TEST_SECTION are not sorted',
291+
'The sections Section_name, SECTION_NAME have duplicate paths',
289292
],
290293
expected_fix=os.path.join(
291294
os.path.dirname(os.path.abspath(__file__)),

‎tests/resources/existing_paths_autofix.txt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ tests/resources/existing_paths_input.txt test1@email.com test2@email.com
1111
/tests/codeowners_linter_tests.py test1@email.com
1212
/tests/resources/empty_input.txt test1@email.com test2@email.com
1313

14-
[SECURITY]
15-
tests/resources/existing_paths_input.txt test1@email.com test2@email.com
14+
[Security]
15+
tests/resources/existing_paths_autofix.txt test1@email.com test2@email.com
16+
tests/resources/existing_paths_input.txt test1@email.com test2@email.com test3@email.com
17+
/tests/resources/existing_paths_autofix.txt test1@email.com test2@email.com
1618

1719
# This is a comment for [SYSTEM]
1820
[SYSTEM]

‎tests/resources/existing_paths_input.txt‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ tests/resources/empty_input.txt test@email.com test3@email.com
1515
/ui/lighting/client test@email.com
1616
tests/resources/existing_paths_input.txt test1@email.com test2@email.com
1717

18-
[SECURITY]
18+
[Security]
1919
/ops/terraform/path1 @test/teams/admin @test/teams/security/admin
2020
/ops/terraform/path2/ @test/teams/security/admin
2121
tests/resources/existing_paths_input.txt test1@email.com test2@email.com
@@ -28,3 +28,15 @@ tests/resources/existing_paths_input.txt test1@email.com test2@email.com
2828
*.py test@email.com
2929
/pipeline/ test@email.com test1@email.com test2@email.com test3@email.com
3030
/release/release_script.sh test@email.com
31+
32+
[SECURITY]
33+
/ops/terraform/path1 @test/teams/admin @test/teams/security/admin
34+
/ops/terraform/path3 @test/teams/admin @test/teams/security/admin
35+
/ops/terraform/path2/ @test/teams/security/admin
36+
tests/resources/existing_paths_input.txt test1@email.com test3@email.com
37+
tests/resources/existing_paths_autofix.txt test1@email.com test2@email.com
38+
39+
[security]
40+
/ops/terraform/path1 @test/teams/admin @test/teams/security/admin
41+
/ops/terraform/path4 @test/teams/admin @test/teams/security/admin
42+
/tests/resources/existing_paths_autofix.txt test1@email.com test2@email.com

‎tests/resources/no_default_section_autofix.txt‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
CODEOWNERS @test/teams/build/admin test@email.com test1@email.com test2@email.com test3@email.com
1515
/ops/gitlab/ test@email.com
1616

17-
# Comment for SECTION_NAME
18-
[SECTION_NAME]
17+
# Comment for Section_name
18+
[Section_name]
1919
* test@email.com test1@email.com test2@email.com test3@email.com
2020
# this is a comment for *.md test@email.com
2121
*.md test@email.com
@@ -32,13 +32,16 @@ WORKSPACE test@email.com test1@email.com test2@email.com test3@email.com
3232
/go/src/github.com/test/path3/ test@email.com
3333
# This is a comment for a non existing line
3434
/go/src/github.com/test/path4/ test@email.com
35-
/ui test@email.com test1@email.com test2@email.com test3@email.com
35+
/ui test1@email.com test2@email.com test3@email.com
3636
/ui/components/ test_a@email.com test_b@email.com test_c@email.com
37+
/ui/components/aaa/ test_c@email.com
3738
/ui/lighting test@email.com
3839
/ui/lighting/client test@email.com
3940
/WORKSPACE test@email.com test1@email.com test2@email.com test3@email.com
4041
/www/ test@email.com
42+
/www/aaa/ test@email.com
4143
/www/gitlab/test/path test@email.com
44+
/www/gitlab/test/path/aaa test@email.com
4245

4346
# Comment for security
4447
[SECURITY]

‎tests/resources/no_default_section_input.txt‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
# - the default section is missing
88
#
99

10-
# Comment for SECTION_NAME
11-
[SECTION_NAME]
10+
# Comment for Section_name
11+
[Section_name]
1212
# this is a comment for *.md test@email.com
1313
*.md test@email.com
1414
* test@email.com test1@email.com test2@email.com test3@email.com
@@ -33,7 +33,7 @@ WORKSPACE test@email.com test1@email.com test2@email.com test3@email.com
3333
/www/gitlab/test/path test@email.com
3434
/ui/components/ test_c@email.com
3535
.gitlab test@email.com
36-
/ui test@email.com test1@email.com test2@email.com test3@email.com
36+
/ui test1@email.com test2@email.com test3@email.com
3737
/ui/components/ test_a@email.com test_b@email.com
3838

3939
# Build Group
@@ -56,6 +56,20 @@ CODEOWNERS @test/teams/build/admin test@email.com test1@email.com test2@email.co
5656
/docker/pipeline/ test@email.com
5757
/release/release_script.sh test@email.com
5858

59+
[SECTION_NAME]
60+
/ui/lighting/client test@email.com
61+
/ui/lighting test@email.com
62+
/ui/components/ test_a@email.com
63+
/www/ test@email.com
64+
/www/gitlab/test/path test@email.com
65+
/ui/components/ test_c@email.com
66+
.gitlab test@email.com
67+
/ui test2@email.com test1@email.com test3@email.com
68+
/ui/components/ test_a@email.com test_b@email.com
69+
/www/aaa/ test@email.com
70+
/www/gitlab/test/path/aaa test@email.com
71+
/ui/components/aaa/ test_c@email.com
72+
5973
[SECURITY]
6074
# Comment for security
6175

@@ -65,6 +79,9 @@ CODEOWNERS @test/teams/build/admin test@email.com test1@email.com test2@email.co
6579
/ops/terraform/path2/ test@email.com
6680

6781

82+
[section_name]
83+
/ui/lighting test@email.com
84+
6885
[TEST_SECTION]
6986
# This is a comment for [TEST_SECTION]
7087

@@ -73,3 +90,6 @@ CODEOWNERS @test/teams/build/admin test@email.com test1@email.com test2@email.co
7390
/pipeline/ test@email.com
7491
/docker/pipeline/ test@email.com
7592
/release/release_script.sh test@email.com
93+
94+
[Section_Name]
95+
/ui/lighting/client test@email.com

0 commit comments

Comments
 (0)