save-cli icon indicating copy to clipboard operation
save-cli copied to clipboard

Support inline fixers for fix and warn plugin

Open orchestr7 opened this issue 4 years ago • 3 comments

We need to support the following LIT fixers: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/test/clang-tidy/checkers/abseil-time-comparison.cpp

// CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
// CHECK-FIXES: absl::FromUnixSeconds(x) <= t1;

We need to:

  1. Support this logic in a fix and warn plugin
  2. To do this we need to generate tmp Expected test resource
  3. Compare files and check results

This should be done under inlineFixer = true option

Keep in mind, that in warn plugin you already have the list with warnings matched to lines, may be it could help you

orchestr7 avatar Aug 31 '21 10:08 orchestr7

Bad news are the following:

  1. I do not know how the following code works:
void g() {
  // CHECK-NOTES: [[@LINE+4]]:5: warning: argument name 'y' in comment does not match parameter name 'x'
  // CHECK-NOTES: [[@LINE-3]]:12: note: 'x' declared here
  // CHECK-NOTES: [[@LINE+2]]:14: warning: argument name 'z' in comment does not match parameter name 'y'
  // CHECK-NOTES: [[@LINE-5]]:19: note: 'y' declared here
  f(/*y=*/0, /*z=*/0);
  // CHECK-FIXES: {{^}}  f(/*y=*/0, /*z=*/0);

  f(/*x=*/1, /*y=*/1);

  ffff(0 /*aaaa=*/, /*bbbb*/ 0); // Unsupported formats.
}
  1. And how the following multi-line fixes are working:
  int VeryVeryVeryVeryLongVariableName;
  absl::Time SomeTime;
  if (some_condition && very_very_very_very_long_variable_name
     < absl::ToUnixSeconds(SomeTime)) {
  // CHECK-MESSAGES: [[@LINE-2]]:25: warning: perform comparison in the time domain [abseil-time-comparison]
  // CHECK-FIXES: if (some_condition && absl::FromUnixSeconds(very_very_very_very_long_variable_name) < SomeTime) {
    return;
  }

orchestr7 avatar Sep 01 '21 13:09 orchestr7

The check_clang_tidy.py script provides an easy way to test both diagnostic messages and fix-its. It filters out CHECK lines from the test file, runs clang-tidy and verifies messages and fixes with two separate FileCheck invocations: once with FileCheck’s directive prefix set to CHECK-MESSAGES, validating the diagnostic messages, and once with the directive prefix set to CHECK-FIXES, running against the fixed code (i.e., the code after generated fix-its are applied). In particular, CHECK-FIXES: can be used to check that code was not modified by fix-its, by checking that it is present unchanged in the fixed code. The full set of FileCheck directives is available (e.g., CHECK-MESSAGES-SAME:, CHECK-MESSAGES-NOT:), though typically the basic CHECK forms (CHECK-MESSAGES and CHECK-FIXES) are sufficient for clang-tidy tests. Note that the FileCheck documentation mostly assumes the default prefix (CHECK), and hence describes the directive as CHECK:, CHECK-SAME:, CHECK-NOT:, etc. Replace CHECK by either CHECK-FIXES or CHECK-MESSAGES for clang-tidy tests.

orchestr7 avatar Sep 01 '21 14:09 orchestr7

@akuleshov7 btw, why it still pinned? I think we can drop it for now

kgevorkyan avatar Feb 17 '23 08:02 kgevorkyan