[clang-tidy][test] Allow empty checks in check_clang_tidy.py

[gamesh411]

Summary

Currently there is no way to assert that a check does not produce warnings for
a specific run. The CHECK-NOT directive can be used, but an empty file is
always discarded by FileCheck. Extend the FileCheck invocation with
'--allow-empty' to support this use-case.

This allows the following usage:

// RUN: %check_clang_tidy %s -check-suffix=CUSTOM <check-name> %t
// CHECK-NOTES-CUSTOM-NOT: NO DIAGS

[gamesh411]

Update commit msg with example

[gamesh411]

Tidy up commit message

[gamesh411]

Do you have some thoughts about this, should this be pursued, or do you think the use-case is not relevant?

[aaron.ballman]

Thank you for the changes, but do you know of any tests that are impacted by this? I'm not opposed to the proposed changes, but I am wondering -- the current behavior is that an empty output file (which can happen with especially egregious bugs) is always an error but if I understand the proposed patch, that may now be silently accepted as a passing test? So I'm wondering if we want to require something like // expected-no-diagnostics from the -verify command in Clang so that users have to explicitly opt a test into the behavior? Or should this not be an issue in general because // CHECK lines should still be present in the test and those will fail when compared against an empty file?

[gamesh411]

In D87830#2298198, @aaron.ballman wrote:

know of any tests that are impacted by this?

I haven't found any tidy-tests that were negative-tests (ie.: tests that assert that there are no diagnostics).

... if I understand the proposed patch, that may now be silently accepted as a passing test?

This is not entirely the case. Let me demonstrate with a few cases:
Before this patch:

// empty test
// RUN: %check_clang_tidy %s %t
// no CHECK lines in the entire file

In the above case the test will fail with CHECK-FIXES, CHECK-MESSAGES or CHECK-NOTES not found in the input.

// empty test, but explicityly mention we dont want diagnostics
// RUN: %check_clang_tidy %s %t
// CHECK_MESSAGES_NOT: ARBITRARY STRING HERE
// ...
// there are *no* other CHECK messages

This passes the first round of sanity checking, but FileCheck nevertheless rejects the processed test file:
FileCheck error: '/home/gamesh411/clang-rwa/tools/clang/tools/extra/test/clang-tidy/checkers/Output/empty.cpp.tmp.cpp.msg' is empty.

// empty test, but explicityly mention we dont want diagnostics
// RUN: %check_clang_tidy %s %t
// CHECK_MESSAGES_NOT: arbitrary string here, I prefer NO DIAG
// ...
// there *are* some other CHECK messages further down

This is fine, but only means that the arbitrary string diagnostic is expected to be *not* seen, while some others are.

After patch:
The second case above becomes fine, everything else stays the same.

Or should this not be an issue in general because // CHECK lines should still be present in the test and those will fail when compared against an empty file?

This means that the only accidental empty test scenario is when someone accidentally skips the CHECK messages he wants to put in, *but* manages to put in the CHECK-NOT by accident as well. This can result from copy-paste, but still, I think this is not likely.

I'm wondering if we want to require something like // expected-no-diagnostics from the -verify command in Clang so that users have to explicitly opt a test into the behavior?

My thoughts were exactly that; I need something like the DiagVerifier's expected-no-diagnostics. That was the motivation behind this patch.

[njames93]

Probably not quite as verbose but should do the job

// RUN: clang-tidy %s --checks=-*,my-check-to-test --warnings-as-errors=* -- -std=c++11

[gamesh411]

In D87830#2336572, @njames93 wrote:

Probably not quite as verbose but should do the job

// RUN: clang-tidy %s --checks=-*,my-check-to-test --warnings-as-errors=* -- -std=c++11

Thanks 👍 ! I can live with this solution as well. If however there is a suggestion on how to handle this in an idiomatic way, please share, maybe I can help implement it.

clang-tools-extra/test/clang-tidy/check_clang_tidy.py

print('------------------------------ Fixes -----------------------------\n' +
         diff_output.decode(errors='ignore') +
         '\n------------------------------------------------------------------')
 
   if has_check_fixes:
     try:
       subprocess.check_output(
           ['FileCheck', '-input-file=' + temp_file_name, input_file_name,
            '-check-prefixes=' + ','.join(check_fixes_prefixes),
-           '-strict-whitespace'],
+           '-strict-whitespace', '--allow-empty'],
           stderr=subprocess.STDOUT)
     except subprocess.CalledProcessError as e:
       print('FileCheck failed:\n' + e.output.decode())
       raise
 
   if has_check_messages:
     messages_file = temp_file_name + '.msg'
     write_file(messages_file, clang_tidy_output)
     try:
       subprocess.check_output(
           ['FileCheck', '-input-file=' + messages_file, input_file_name,
            '-check-prefixes=' + ','.join(check_messages_prefixes),
-           '-implicit-check-not={{warning|error}}:'],
+           '-implicit-check-not={{warning|error}}:', '--allow-empty'],
           stderr=subprocess.STDOUT)
     except subprocess.CalledProcessError as e:
       print('FileCheck failed:\n' + e.output.decode())
       raise
 
   if has_check_notes:
     notes_file = temp_file_name + '.notes'
     filtered_output = [line for line in clang_tidy_output.splitlines()
                        if not "note: FIX-IT applied" in line]
     write_file(notes_file, '\n'.join(filtered_output))
     try:
       subprocess.check_output(
           ['FileCheck', '-input-file=' + notes_file, input_file_name,
            '-check-prefixes=' + ','.join(check_notes_prefixes),
-           '-implicit-check-not={{note|warning|error}}:'],
+           '-implicit-check-not={{note|warning|error}}:', '--allow-empty'],
           stderr=subprocess.STDOUT)
     except subprocess.CalledProcessError as e:
       print('FileCheck failed:\n' + e.output.decode())
       raise
 
 
 def expand_std(std):
   if std == 'c++98-or-later':