[clang-tidy] Fix extern fixes in readability-redundant-declaration

[PiotrZSL]

Summary

Currently check does not look into LinkageSpecDecl
when removing redundant variable re-declarations.
This were leaving code in non-compiling state.
Fix patch fixes this and adds removal also of 'extern C'.

Fixes: https://github.com/llvm/llvm-project/issues/42068

[Eugene.Zelenko]

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

84

auto could be used because type is explicitly stated in same statement.

[PiotrZSL]

Use auto

[carlosgalvezp]

Looks good, thanks for the fix! Do we think it's worth documenting in the Release Notes?

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

86

This can be removed, leaving it as:

if (const auto *Extern = ... && !Extern->hasBraces())

clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp

126

Typo: dummy

I don't quite see why these are needed to test the example in PR42068?

[PiotrZSL]

clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp

126

I wasn't sure about getLocForEndOfToken, so just wanted to add some tokens before and after, to be 100% sure that they wont be removed.

[PiotrZSL]

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

86

If you mean:

 if (const auto *Extern =
              Result.Nodes.getNodeAs<LinkageSpecDecl>("extern");
          !Extern->hasBraces())
`

then no, it cannot be like this because Extern can be null.

[PiotrZSL]

Added release notes, fixed typo.

[carlosgalvezp]

Looks great, thanks!

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

86

You are right, this can only be done when there's no other conditions in the if!

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

 }
 
 void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       namedDecl(anyOf(varDecl(unless(isDefinition())),
                       functionDecl(unless(anyOf(
                           isDefinition(), isDefaulted(),
                           doesDeclarationForceExternallyVisibleDefinition(),
-                          hasAncestor(friendDecl()))))))
+                          hasAncestor(friendDecl()))))),
+                optionally(hasParent(linkageSpecDecl().bind("extern"))))
           .bind("Decl"),
       this);
 }
 
 void RedundantDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *D = Result.Nodes.getNodeAs<NamedDecl>("Decl");
   const auto *Prev = D->getPreviousDecl();
   if (!Prev)
for (const auto *Other : VD->getDeclContext()->decls()) {
       }
     }
   }
 
   SourceLocation EndLoc = Lexer::getLocForEndOfToken(
       D->getSourceRange().getEnd(), 0, SM, Result.Context->getLangOpts());
   {
     auto Diag = diag(D->getLocation(), "redundant %0 declaration") << D;
-    if (!MultiVar && !DifferentHeaders)
-      Diag << FixItHint::CreateRemoval(
-          SourceRange(D->getSourceRange().getBegin(), EndLoc));
+    if (!MultiVar && !DifferentHeaders) {
+      SourceLocation BeginLoc;
+      if (const auto *Extern =
+              Result.Nodes.getNodeAs<LinkageSpecDecl>("extern");
+          Extern && !Extern->hasBraces())
+        BeginLoc = Extern->getExternLoc();
+      else
+        BeginLoc = D->getSourceRange().getBegin();
+
+      Diag << FixItHint::CreateRemoval(SourceRange(BeginLoc, EndLoc));
+    }
   }
   diag(Prev->getLocation(), "previously declared here", DiagnosticIDs::Note);
 }
 } // namespace clang::tidy::readability

clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp

 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
 // CHECK-FIXES: {{^}}// g{{$}}
 
 #if defined(EXTERNINLINE)
 extern inline void g(); // extern g
 // CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' declaration
 // CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
 #endif
+
+// PR42068
+extern "C" int externX;
+int dumyBegin;extern "C" int externX;int dummyEnd;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: redundant 'externX' declaration [readability-redundant-declaration]
+// CHECK-FIXES: {{^}}int dumyBegin;int dummyEnd;{{$}}