[PATCH] D96542: [clang-tidy] Fix `TransformerClangTidyCheck`'s handling of include insertions.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:108
+      Diag << Inserter.createIncludeInsertion(
+          Result.SourceManager->getFileID(T.Range.getBegin()), T.Replacement);
       break;
----------------
alexfh wrote:
> Can this be a macro file id? I'd suggest to add tests (probably for checks using this functionality) with a few nested includes and fixes in normal code, code in macros declared and expanded in different files, locations in macro bodies, macro arguments, and some tricky cases like fix pointing to a pasted token.
> Can this be a macro file id? I'd suggest to add tests (probably for checks using this functionality) with a few nested includes and fixes in normal code, code in macros declared and expanded in different files, locations in macro bodies, macro arguments, and some tricky cases like fix pointing to a pasted token.

Unlikely, given that API that populates the location. Most use a default verion which explicitly maps the location to the expansion location. However, it is possible. Also, Transformer rejects changes inside macros (except arguments) so that would also be hard to engineer.

Still, I'll look into better tests in the check itself. I did try that but mostly failed because the lit tests don't support checking anything in headers. However, the macro checks sound worth it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96542/new/

https://reviews.llvm.org/D96542