Make add_new_check.py's insertion of registerCheck<> match the sort order

[dsanders]

Summary

Following on from review comments in D65919 about the ordering
of the registerCheck<> calls. Sort the calls based on the check name
string

[aaron.ballman]

Given that the alphabetization we want really is based on the string literal, would it make sense to look for that rather than the check name? Adding a few more reviewers for a better mix of opinions.

[alexfh]

In D66505#1641776, @aaron.ballman wrote:

Given that the alphabetization we want really is based on the string literal, would it make sense to look for that rather than the check name? Adding a few more reviewers for a better mix of opinions.

Ideally, it would be nice to sort the list of registrations by the check name. But given that the call can span two lines, that may be slightly trickier to implement. It's worth giving it a try though ;)

[dsanders]

Sort on the check name string

[alexfh]

Mostly LG, if you've verified that this works. A couple of comments below.

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

193

Are there actually any registerCheck calls with spaces between the > and the (? Should we clang-format these instead of supporting them in the script?

194

I'd call this previous_line or prev_line. "Last" is a bit misleading here.

[aaron.ballman]

Generally LG to me as well

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

199

Missing a full stop at the end of the comment.

[dsanders]

In D66505#1652420, @alexfh wrote:

Mostly LG, if you've verified that this works. A couple of comments below.

I verified it using

./add_new_check.py llvm prefer-register-over-unsigned
./add_new_check.py llvm zzz
./add_new_check.py llvm nb
./add_new_check.py llvm iz
./add_new_check.py llvm a

which is enough to cover every insertion point in the LLVM module

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

193

Are there actually any registerCheck calls with spaces between the > and the (?

No, it was just trivial to be robust against it in case it does somehow happen. Likewise for the whitespace after the '(' but before the '"' or newline.

Should we clang-format these instead of supporting them in the script?

I think the script should stick to inserting the new code rather than correcting existing mistakes that somehow crept in. I do agree that people should generally clang-format their patches before commit though which will prevent it creeping in.

[dsanders]

• Full stop at end of comment
• last_line -> prev_line

[dsanders]

Missed one of the commands from my history that I used to verify it:

./add_new_check.py llvm i

[aaron.ballman]

LGTM, thank you for working on this!

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

for line in lines:
           header_added = True
           f.write('#include "' + check_name_camel + '.h"\n')
 
       if not check_added:
         if line.strip() == '}':
           check_added = True
           f.write(check_decl)
         else:
-          match = re.search('registerCheck<(.*)>', line)
+          match = re.search('registerCheck<((.*::).*)>', line)
           if match and match.group(1) > check_name_camel:
             check_added = True
             f.write(check_decl)
       f.write(line)
 
 
 # Adds a release notes entry.
 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('Improvements to clang-tidy')
   nextSectionMatcher = re.compile('Improvements to clang-include-fixer')
   checkerMatcher = re.compile('- New :doc:`(.*)')
 
   print('Updating %s...' % filename)