- fopen() should include "e" in their mode string. [android-fopen-mode]
Is there a guideline/code style describing it? If so, would be nice to add the link.
And I'd suggest you to put the detailed document to the rst instead of code comments -- because users usually read documents.
nit: remove the blank line.
No need to put utility functions/variables inside the class, you can define them in an anonymous namespace in the implementation file, the same below.
nit: remove extra trailing ===
Use completed statements fopen("filename", "re"); to do the fix verification. The same below.
You can use a shorter string here -- It is sufficient as we have a completed message check in the first CHECK-MESSAGES.
Adding 'e' is like adding the "O_CLOEXEC". The reason is the same with part1(https://reviews.llvm.org/D33304). Added more text in doc.
Inline one and move the other and the variable to anonymous namespace.
The fix will mark the corresponding argument and suggest the fix like:
Is this clear to show the fix?
We may not need this utility function, see my comment on your part 2.
How about if (const auto* ModeStr = dyn_cast<StringLiteral>(...))?
remove the trailing ., clang-tidy message is not a sentence.
And we can simplify the code like: diag(...) << FD << FixItHint::CreateReplacement(ModeRange.getSourceRange(), ReplacementText);
CHECK-FIXES is not used to check the message of the fix. It is used to check the code after the generated-fix is applied. So here should be // CHECK-FIXES: fopen("filename", "re");.
You can use Twine here which is more efficient: (getSourceText(...) + " \"" + Twine(Mode) + "\"").str().
The same below.
We'd prefer early return in llvm, and with that the flag "MayHaveE" can be removed.
I think you can use ModeArg->getSourceRange().
Moreover, as ModeRange is used only once, you can ModeArg->getSourceRange() directly in FixtItHint below.