[clang-tidy] Fix readability-trailing-comma after multi-line comments

clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp

@@ -56,6 +56,36 @@ static bool isPreprocessorDirectiveToken(const Token &Tok,
              SM.getExpansionLineNumber(Tok.getLocation());
 }
 
+// Returns the last non-comment token before the closing brace of Enum.
+// Lexer::findPreviousToken() can't be used for this: it starts lexing at the
+// beginning of a line, which may be inside a multi-line block comment.
+static std::optional<Token> findLastEnumToken(const EnumDecl *Enum,
+                                              const SourceManager &SM,
+                                              const LangOptions &LangOpts) {
+  const auto [LBraceFile, LBraceOffset] =
+      SM.getDecomposedExpansionLoc(Enum->getBraceRange().getBegin());
+  const auto [RBraceFile, RBraceOffset] =
+      SM.getDecomposedExpansionLoc(Enum->getBraceRange().getEnd());
+  bool Invalid = false;
+  const StringRef Buffer = SM.getBufferData(RBraceFile, &Invalid);
+  if (Invalid)
+    return std::nullopt;
+  // The braces are in different files if the enum's body is split across an
+  // #include. Then lex the closing brace's file from its beginning.
+  const bool SameFile = LBraceFile == RBraceFile;
+  Lexer RawLexer(SM.getLocForStartOfFile(RBraceFile), LangOpts, Buffer.begin(),
+                 Buffer.data() + (SameFile ? LBraceOffset : 0), Buffer.end());
+  Token Tok;
+  // Skip the opening brace.
+  if (SameFile && RawLexer.LexFromRawLexer(Tok))
+    return std::nullopt;
+  std::optional<Token> LastTok;
+  while (!RawLexer.LexFromRawLexer(Tok) &&
+         SM.getFileOffset(Tok.getLocation()) < RBraceOffset)
+    LastTok = Tok;
+  return LastTok;
+}
+
 namespace {
 
 AST_POLYMORPHIC_MATCHER(isMacro,
@@ -122,9 +152,8 @@ void TrailingCommaCheck::checkEnumDecl(const EnumDecl *Enum,
   if (Policy == CommaPolicyKind::Ignore)
     return;
 
-  const std::optional<Token> LastTok = Lexer::findPreviousToken(
-      Enum->getBraceRange().getEnd(), *Result.SourceManager, getLangOpts(),
-      /*IncludeComments=*/false);
+  const std::optional<Token> LastTok =
+      findLastEnumToken(Enum, *Result.SourceManager, getLangOpts());
   if (!LastTok)
     return;
 

clang-tools-extra/docs/ReleaseNotes.md

@@ -473,6 +473,9 @@ infrastructure are described first, followed by tool-specific sections.
   - Fixed a false positive on empty brace initializers of types with default
     member initializers.
 
+  - Fixed wrong diagnostics and fix-its when an enum's last enumerator is
+    followed by a block comment that spans multiple lines.
+
 - Improved {doc}`readability-use-std-min-max
   <clang-tidy/checks/readability/use-std-min-max>` check by fixing spurious
   trailing semicolons and lost comments when the `if` body has no braces.

clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/trailing-comma/split-enum.h

@@ -0,0 +1,2 @@
+enum SplitEnum {
+  SplitA,

clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-remove.cpp

@@ -33,6 +33,17 @@ enum E3 {
 };
 enum Empty {};
 
+// A multi-line block comment before '}' is skipped.
+enum MultiLineComment {
+  MLC_A, /* comment that
+            spans multiple lines */
+};
+// CHECK-MESSAGES: :[[@LINE-3]]:8: warning: enum should not have a trailing comma
+// CHECK-FIXES: enum MultiLineComment {
+// CHECK-FIXES-NEXT:   MLC_A /* comment that
+// CHECK-FIXES-NEXT:            spans multiple lines */
+// CHECK-FIXES-NEXT: };
+
 void f() {
   // Multi-line init lists with trailing commas - should warn to remove
   int a[] = {

clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-split-enum.cpp

@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s readability-trailing-comma %t -- \
+// RUN:   -- -I %S/Inputs/trailing-comma
+
+// The enum's opening brace is in the header and its closing brace is here.
+#include "split-enum.h"
+  SplitB
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: enum should have a trailing comma
+// CHECK-FIXES: SplitB,
+// CHECK-FIXES-NEXT: };

clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp

@@ -190,6 +190,65 @@ enum NullDirectiveAfter {
 #
 };
 
+// A multi-line block comment before '}' is skipped.
+enum MultiLineCommentAfterComma {
+  MLC_A, /* comment that
+            spans multiple lines */
+};
+
+enum MultiLineCommentNoComma {
+  MLC_B /* comment that
+           spans multiple lines */
+};
+// CHECK-MESSAGES: :[[@LINE-3]]:8: warning: enum should have a trailing comma
+// CHECK-FIXES: enum MultiLineCommentNoComma {
+// CHECK-FIXES-NEXT:   MLC_B, /* comment that
+// CHECK-FIXES-NEXT:            spans multiple lines */
+// CHECK-FIXES-NEXT: };
+
+// Enumerators from macros are checked like other enumerators.
+#define ENUMERATOR(n) n
+enum MacroEnumerator {
+  ENUMERATOR(ME_A),
+  ENUMERATOR(ME_B)
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: enum should have a trailing comma
+// CHECK-FIXES: enum MacroEnumerator {
+// CHECK-FIXES-NEXT:   ENUMERATOR(ME_A),
+// CHECK-FIXES-NEXT:   ENUMERATOR(ME_B),
+// CHECK-FIXES-NEXT: };
+
+enum MacroEnumeratorMultiLineComment {
+  ENUMERATOR(MEC_A),
+  ENUMERATOR(MEC_B) /* comment that
+                       spans multiple lines */
+};
+// CHECK-MESSAGES: :[[@LINE-3]]:20: warning: enum should have a trailing comma
+// CHECK-FIXES: enum MacroEnumeratorMultiLineComment {
+// CHECK-FIXES-NEXT:   ENUMERATOR(MEC_A),
+// CHECK-FIXES-NEXT:   ENUMERATOR(MEC_B), /* comment that
+// CHECK-FIXES-NEXT:                        spans multiple lines */
+// CHECK-FIXES-NEXT: };
+
+// The opening brace can come from a macro.
+#define LBRACE {
+enum MacroOpeningBrace LBRACE
+  MOB_A,
+  MOB_B
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:8: warning: enum should have a trailing comma
+// CHECK-FIXES: enum MacroOpeningBrace LBRACE
+// CHECK-FIXES-NEXT:   MOB_A,
+// CHECK-FIXES-NEXT:   MOB_B,
+// CHECK-FIXES-NEXT: };
+
+// An enum whose closing brace comes from a macro is ignored.
+#define RBRACE }
+enum MacroClosingBrace {
+  MCB_A,
+  MCB_B
+RBRACE;
+
 // Macros are ignored
 #define ENUM(n, a, b) enum n { a, b }
 #define INIT {1, 2}