[clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

[Eugene.Zelenko]

Summary

Also use check in add_new_check.py for terminology consistency.

PS

My GitHub ID is EugeneZelenko, if it's necessary for attribution.

[njames93]

It looks good, but could you maybe create a child revision showing what it looks like with a few checks renamed to make sure everything is all working correctly.

[Eugene.Zelenko]

In D73580#1852032, @njames93 wrote:

It looks good, but could you maybe create a child revision showing what it looks like with a few checks renamed to make sure everything is all working correctly.

I think it'll be easier to test script locally, since several (at least two) child revisions would be necessary. By the word, main script boy is borrowed from add_new_check.py, so difference only in section name and regular expression for entry.

[njames93]

So I tried to ran ./rename_check.py readability-braces-around-statements readability-braces and results were less than desirable. It renamed the readability-braces-around-statements but it also renamed the google alias to google-readability-braces. In the documentation for google-readability-braces it changed the subject line to

google-readability-braces
===========================================

Feel like the correct course of action is to match against the entire check name rather than a sub string of it (which i guess was unintentional). This doesn't need to be done in this review and could (should) be a follow up.

The actual alphabetical ordering of the release notes works as intended though.

[alexfh]

clang-tools-extra/clang-tidy/rename_check.py

172–174

What's the preferred variable naming convention in Python in LLVM? Looking at the two scripts now, I see that the naming style is already quite inconsistent. It would be nice to make naming consistent in these scripts.

Not an action item for this patch, just a drive-by comment.

[Eugene.Zelenko]

Another side effect is that rename_check.py messes list.rst by removing "Offer fixes". Probably proper solution would be moving update_checks_list from add_new_check.py to shared module.

[njames93]

LGTM, For what this is trying to achieve I have no issue, follow up patches can be submitted for the other enhancements needed.

[Eugene.Zelenko]

In D73580#1883861, @njames93 wrote:

LGTM, For what this is trying to achieve I have no issue, follow up patches can be submitted for the other enhancements needed.

Please commit patch if you'll have time. See summary for details.

[njames93]

In D73580#1884042, @Eugene.Zelenko wrote:

In D73580#1883861, @njames93 wrote:

LGTM, For what this is trying to achieve I have no issue, follow up patches can be submitted for the other enhancements needed.

Please commit patch if you'll have time. See summary for details.

I figured it out, arc patch doesn't seem to handle the author of the commit, but I could use git commit amend for that

clang-tools-extra/clang-tidy/add_new_check.py

def add_release_notes(module_path, module, check_name):
   check_name_dashes = module + '-' + check_name
   filename = os.path.normpath(os.path.join(module_path,
                                            '../../docs/ReleaseNotes.rst'))
   with open(filename, 'r') as f:
     lines = f.readlines()
 
   lineMatcher = re.compile('New checks')
   nextSectionMatcher = re.compile('New check aliases')
-  checkerMatcher = re.compile('- New :doc:`(.*)')
+  checkMatcher = re.compile('- New :doc:`(.*)')
 
   print('Updating %s...' % filename)
   with open(filename, 'w') as f:
     note_added = False
     header_found = False
     next_header_found = False
     add_note_here = False
 
     for line in lines:
       if not note_added:
         match = lineMatcher.match(line)
         match_next = nextSectionMatcher.match(line)
-        match_checker = checkerMatcher.match(line)
-        if match_checker:
-          last_checker = match_checker.group(1)
-          if last_checker > check_name_dashes:
+        match_check = checkMatcher.match(line)
+        if match_check:
+          last_check = match_check.group(1)
+          if last_check > check_name_dashes:
             add_note_here = True
 
         if match_next:
           next_header_found = True
           add_note_here = True
 
         if match:
           header_found = True

clang-tools-extra/clang-tidy/rename_check.py

 
 # Adds a release notes entry.
 def add_release_notes(clang_tidy_path, old_check_name, new_check_name):
   filename = os.path.normpath(os.path.join(clang_tidy_path,
                                            '../docs/ReleaseNotes.rst'))
   with open(filename, 'r') as f:
     lines = f.readlines()
 
+  lineMatcher = re.compile('Renamed checks')
+  nextSectionMatcher = re.compile('Improvements to include-fixer')
+  checkMatcher = re.compile('- The \'(.*)')
+
   print('Updating %s...' % filename)
   with open(filename, 'wb') as f:
     note_added = False
     header_found = False
+    next_header_found = False
+    add_note_here = False
 
     for line in lines:
       if not note_added:
-        match = re.search('Renamed checks', line)
+        match = lineMatcher.match(line)
+        match_next = nextSectionMatcher.match(line)
+        match_check = checkMatcher.match(line)
+        if match_check:
+          last_check = match_check.group(1)
+          if last_check > old_check_name:
+            add_note_here = True
+
+        if match_next:
+          next_header_found = True
+          add_note_here = True
+
         if match:
           header_found = True
-        elif header_found:
+          f.write(line)
+          continue
+
+        if line.startswith('^^^^'):
+          f.write(line)
+          continue
+
+        if header_found and add_note_here:
           if not line.startswith('^^^^'):
-            f.write("""
-- The '%s' check was renamed to :doc:`%s
+            f.write("""- The '%s' check was renamed to :doc:`%s
   <clang-tidy/checks/%s>`
+
 """ % (old_check_name, new_check_name, new_check_name))
             note_added = True
 
       f.write(line)
 
 def main():
   parser = argparse.ArgumentParser(description='Rename clang-tidy check.')
   parser.add_argument('old_check_name', type=str,

clang-tools-extra/docs/ReleaseNotes.rst

- Improved :doc:`readability-redundant-string-init
   <clang-tidy/checks/readability-redundant-string-init>` check now supports a
   `StringNames` option enabling its application to custom string classes. The
   check now detects in class initializers and constructor initializers which
   are deemed to be redundant.
 
 Renamed checks
 ^^^^^^^^^^^^^^
 
-
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 
 Improvements to clang-include-fixer
 -----------------------------------