[PATCH] D142123: [clang-tidy] Add header guard style to suggest use of #pragma once

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

LGTM assuming precommit CI doesn't discover any surprises, but hopefully @njames93 or @carlosgalvezp can weigh in as well since they've been in this code more recently than I have.



================
Comment at: clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp:569-571
+            runPragmaOnceCheck("#ifndef BOTH_H\n"
+                               "#define BOTH_H\n"
+                               "#pragma once\n"
----------------
KyleFromKitware wrote:
> aaron.ballman wrote:
> > How about a test for:
> > ```
> > #if !defined(HEADER_GUARD_H)
> > #define HEADER_GUARD_H 1
> > 
> > #endif
> > ```
> > and a test for what happens when there's no header guard or pragma once in the file.
> > How about a test for:
> > ```
> > #if !defined(HEADER_GUARD_H)
> > #define HEADER_GUARD_H 1
> > 
> > #endif
> > ```
> 
> Good catch. The case of `!defined()` was not being handled by the existing header guard check except to basically ignore it. The `#pragma once` check crashed on this case. I've updated it to also ignore the `!defined()` case.
> 
> > and a test for what happens when there's no header guard or pragma once in the file.
> 
> I already have this, see the "neither" test at the bottom.
>> and a test for what happens when there's no header guard or pragma once in the file.
> I already have this, see the "neither" test at the bottom.

Ah! I was misreading:
```
  EXPECT_EQ("#pragma once\n"
            "void neither();\n",
            runPragmaOnceCheck("void neither();\n", "neither.h",
                               StringRef("use #pragma once")));
```
as having the pragma once in there, which is backwards. Thanks!


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

https://reviews.llvm.org/D142123