Fix add-copyright year function (#466)

* Fix add-copyright year function * Fixes bug raised in [#414](#414) Signed-off-by: superdyoll <[email protected]>
ament · Mar 3, 2024 · 2d4f46c · 2d4f46c
1 parent 4bd8e5b
commit 2d4f46c
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 46 deletions.
diff --git a/ament_copyright/ament_copyright/main.py b/ament_copyright/ament_copyright/main.py
@@ -267,14 +267,14 @@ def add_missing_header(file_descriptors, name, license_, verbose):
 
 def add_copyright_year(file_descriptors, new_years, verbose):
     if verbose:
-        print('Adding the current year to existing copyright notices:')
+        print(f'Adding {",".join(map(str,new_years))} to existing copyright notices:')
         print()
 
     for path in sorted(file_descriptors.keys()):
         file_descriptor = file_descriptors[path]
 
         # ignore files which do not have a header
-        if not getattr(file_descriptor, 'copyright_identifier', None):
+        if not getattr(file_descriptor, 'copyright_identifiers', None):
             continue
 
         index = scan_past_coding_and_shebang_lines(file_descriptor.content)
@@ -287,39 +287,32 @@ def add_copyright_year(file_descriptors, new_years, verbose):
         else:
             block = file_descriptor.content[index:]
             block_offset = 0
-        copyright_span, years_span, name_span = search_copyright_information(block)
-        if copyright_span is None:
+        copyrights, years_spans, _, _ = search_copyright_information(block)
+        if copyrights is None:
             assert False, "Could not find copyright information in file '%s'" % \
                 file_descriptor.path
 
-        # skip if all new years are already included
-        years = get_years_from_string(block[years_span[0]:years_span[1]])
-        if all((new_year in years) for new_year in new_years):
-            if verbose:
-                print(' ', file_descriptor.path)
-            continue
-        print('*' if file_descriptor.exists else '+', file_descriptor.path)
-
-        for new_year in new_years:
-            years.add(new_year)
-        years_string = get_string_from_years(years)
-
-        # overwrite previous years with new years
-        offset = index + block_offset
-        global_years_span = [offset + years_span[0], offset + years_span[1]]
-        content = file_descriptor.content[:global_years_span[0]] + years_string + \
-            file_descriptor.content[global_years_span[1]:]
-
-        # output beginning of file for debugging
-        # index = global_years_span[0]
-        # for _ in range(3):
-        #     index = get_index_of_next_line(content, index)
-        # print('<<<')
-        # print(content[:index - 1])
-        # print('>>>')
-
-        with open(file_descriptor.path, 'w', encoding='utf-8') as h:
-            h.write(content)
+        for years_span in years_spans:
+            # skip if all new years are already included
+            years = get_years_from_string(block[years_span[0]:years_span[1]])
+            if all((new_year in years) for new_year in new_years):
+                if verbose:
+                    print(' ', file_descriptor.path)
+                continue
+            print('*' if file_descriptor.exists else '+', file_descriptor.path)
+
+            for new_year in new_years:
+                years.add(new_year)
+            years_string = get_string_from_years(years)
+
+            # overwrite previous years with new years
+            offset = index + block_offset
+            global_years_span = [offset + years_span[0], offset + years_span[1]]
+            content = file_descriptor.content[:global_years_span[0]] + years_string + \
+                file_descriptor.content[global_years_span[1]:]
+
+            with open(file_descriptor.path, 'w', encoding='utf-8') as h:
+                h.write(content)
 
 
 def get_years_from_string(content):

diff --git a/ament_copyright/ament_copyright/parser.py b/ament_copyright/ament_copyright/parser.py
@@ -115,11 +115,11 @@ def parse(self):
 
         # get first comment block without leading comment tokens
         block, _ = get_comment_block(self.content, index)
-        copyrights, remaining_block = search_copyright_information(block)
+        copyrights, _, _, remaining_block = search_copyright_information(block)
 
         if len(copyrights) == 0:
             block = get_multiline_comment_block(self.content, index)
-            copyrights, remaining_block = search_copyright_information(block)
+            copyrights, _, _, remaining_block = search_copyright_information(block)
 
         if len(copyrights) == 0:
             return
@@ -193,17 +193,21 @@ def search_copyright_information(content):
     regex = re.compile(pattern, re.DOTALL | re.MULTILINE | re.IGNORECASE)
 
     copyrights = []
+    years_spans = []
+    name_spans = []
     while True:
         match = regex.search(content)
         if not match:
             break
         years_span, name_span = match.span(1), match.span(2)
+        years_spans.append(years_span)
+        name_spans.append(name_span)
         years = content[years_span[0]:years_span[1]]
         name = content[name_span[0]:name_span[1]]
         copyrights.append(CopyrightDescriptor(name, years))
         content = content[name_span[1]:]
 
-    return copyrights, content
+    return copyrights, years_spans, name_spans, content
 
 
 def scan_past_coding_and_shebang_lines(content):

diff --git a/ament_copyright/test/test_parser.py b/ament_copyright/test/test_parser.py
@@ -23,27 +23,36 @@
 
 def test_search_copyright_information_incorrect_typo():
     """Test searching for copyright information with a typo in the copyright information."""
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         'CopyrightTypo 2020 Open Source Robotics Foundation, Inc.'
     )
     assert len(copyrights) == 0
+    assert years_span == []
+    assert name_span == []
+    assert len(remaining_block) == 56
 
 
 def test_search_copyright_information_repeated():
     """Test searching with repeated 'copyright' in the copyright information."""
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         '\\copyright Copyright 2020 Open Source Robotics Foundation, Inc.'
     )
     assert len(copyrights) == 1
+    assert years_span == [(21, 25)]
+    assert name_span == [(26, 63)]
+    assert len(remaining_block) == 0
 
 
 def test_search_copyright_information_multiple_holders():
     """Test searching multiple holders."""
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         """Copyright 2020 Open Source Robotics Foundation, Inc.
            Copyright (c) 2009, Willow Garage, Inc."""
     )
     assert len(copyrights) == 2
+    assert years_span == [(10, 14), (26, 30)]
+    assert name_span == [(15, 52), (32, 51)]
+    assert len(remaining_block) == 0
 
 
 def test_search_copyright_information_capitalization1():
@@ -53,10 +62,13 @@ def test_search_copyright_information_capitalization1():
     Word 'copyright': capitalized
     Abbreviation '(c)': absent
     """
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         '  Copyright 2020 Open Source Robotics Foundation, Inc.')
-    assert copyrights[0].name == 'Open Source Robotics Foundation, Inc.'
     assert len(copyrights) == 1
+    assert copyrights[0].name == 'Open Source Robotics Foundation, Inc.'
+    assert years_span == [(12, 16)]
+    assert name_span == [(17, 54)]
+    assert len(remaining_block) == 0
 
 
 def test_search_copyright_information_capitalization2():
@@ -66,9 +78,12 @@ def test_search_copyright_information_capitalization2():
     Word 'copyright': capitalized
     Abbreviation '(c)': lowercase
     """
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         'Copyright (c) 2020 Open Source Robotics Foundation, Inc.')
     assert len(copyrights) == 1
+    assert years_span == [(14, 18)]
+    assert name_span == [(19, 56)]
+    assert len(remaining_block) == 0
 
 
 def test_search_copyright_information_capitalization3():
@@ -78,9 +93,12 @@ def test_search_copyright_information_capitalization3():
     Word 'copyright': capitalized
     Abbreviation '(c)': uppercase
     """
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         'Copyright (C) 2020 Open Source Robotics Foundation, Inc.')
     assert len(copyrights) == 1
+    assert years_span == [(14, 18)]
+    assert name_span == [(19, 56)]
+    assert len(remaining_block) == 0
 
 
 def test_search_copyright_information_lowercase1():
@@ -90,9 +108,12 @@ def test_search_copyright_information_lowercase1():
     Word 'copyright': lowercase
     Abbreviation '(c)': absent
     """
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         'copyright 2020 Open Source Robotics Foundation, Inc.')
     assert len(copyrights) == 1
+    assert years_span == [(10, 14)]
+    assert name_span == [(15, 52)]
+    assert len(remaining_block) == 0
 
 
 def test_search_copyright_information_lowercase2():
@@ -102,9 +123,12 @@ def test_search_copyright_information_lowercase2():
     Word 'copyright': lowercase
     Abbreviation '(c)': lowercase
     """
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         'copyright (c) 2020 Open Source Robotics Foundation, Inc.')
     assert len(copyrights) == 1
+    assert years_span == [(14, 18)]
+    assert name_span == [(19, 56)]
+    assert len(remaining_block) == 0
 
 
 def test_search_copyright_information_uppercase1():
@@ -114,9 +138,12 @@ def test_search_copyright_information_uppercase1():
     Word 'copyright': uppercase
     Abbreviation '(c)': absent
     """
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         'COPYRIGHT 2020 Open Source Robotics Foundation, Inc.')
     assert len(copyrights) == 1
+    assert years_span == [(10, 14)]
+    assert name_span == [(15, 52)]
+    assert len(remaining_block) == 0
 
 
 def test_search_copyright_information_uppercase2():
@@ -126,9 +153,12 @@ def test_search_copyright_information_uppercase2():
     Word 'copyright': uppercase
     Abbreviation '(c)': uppercase
     """
-    copyrights, remaining_block = search_copyright_information(
+    copyrights, years_span, name_span, remaining_block = search_copyright_information(
         'COPYRIGHT (C) 2020 Open Source Robotics Foundation, Inc.')
     assert len(copyrights) == 1
+    assert years_span == [(14, 18)]
+    assert name_span == [(19, 56)]
+    assert len(remaining_block) == 0
 
 
 def test_split_template_no_split():