Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support inline fixers for fix and warn plugin #221

Open
orchestr7 opened this issue Aug 31, 2021 · 3 comments
Open

Support inline fixers for fix and warn plugin #221

orchestr7 opened this issue Aug 31, 2021 · 3 comments
Assignees
Labels
backlog enhancement New feature or request
Milestone

Comments

@orchestr7
Copy link
Member

orchestr7 commented Aug 31, 2021

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 orchestr7 added the enhancement New feature or request label Aug 31, 2021
@orchestr7 orchestr7 added this to the 0.2.0 milestone Aug 31, 2021
Cheshiriks added a commit that referenced this issue Aug 31, 2021
What's done:
* Added support inline fixers for fix and warn plugin
Closes #221
@orchestr7 orchestr7 pinned this issue Sep 1, 2021
@orchestr7
Copy link
Member Author

orchestr7 commented Sep 1, 2021

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
Copy link
Member Author

orchestr7 commented Sep 1, 2021

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.

Cheshiriks added a commit that referenced this issue Sep 2, 2021
What's done:
* Added support inline fixers for fix and warn plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 2, 2021
What's done:
* Added support inline fixers for fix and warn plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 3, 2021
What's done:
* Added support inline fixers for fix and warn plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 3, 2021
What's done:
* Added support inline fixers for fix and warn plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 3, 2021
What's done:
* Change test storage from List to class in plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 3, 2021
What's done:
* Change test storage from List to class in plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 3, 2021
What's done:
* Change test storage from List to class in plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 3, 2021
What's done:
* Change test storage from List to class in plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 3, 2021
What's done:
* Change test storage from List to class in plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 3, 2021
What's done:
* Change test storage from List to class in plugin
Closes #221
Cheshiriks added a commit that referenced this issue Sep 3, 2021
What's done:
* Change test storage from List to class in plugin
Closes #221
@kgevorkyan
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants