[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/docs/ReleaseNotes.rst

- Improved :doc:`readability-magic-numbers
   <clang-tidy/checks/readability/magic-numbers>` check, now allows for
   magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
   the new ``IgnoreTypeAliases`` option is set to true.
 
 - Fixed a false positive in :doc:`readability-misleading-indentation
   <clang-tidy/checks/readability/misleading-indentation>` check when warning would
   be unnecessarily emitted for template dependent ``if constexpr``.
 
+- Fixed incorrect fixes in :doc:`readability-redundant-declaration
+  <clang-tidy/checks/readability/redundant-declaration>` check when linkage
+  (like ``extern "C"``) is explicitly specified.
+
 - Improved :doc:`readability-static-accessed-through-instance
   <clang-tidy/checks/readability/static-accessed-through-instance>` check to
   support unscoped enumerations through instances and fixed usage of anonymous
   structs or classes.
 
 - Fixed a false positive in :doc:`cppcoreguidelines-slicing
   <clang-tidy/checks/cppcoreguidelines/slicing>` check when warning would be
   emitted in constructor for virtual base class initialization.
 
 - Improved :doc:`bugprone-use-after-move
   <clang-tidy/checks/bugprone/use-after-move>` to understand that there is a
   sequence point between designated initializers.
 
 - Fixed an issue in :doc:`readability-identifier-naming
   <clang-tidy/checks/readability/identifier-naming>` when specifying an empty
   string for ``Prefix`` or ``Suffix`` options could result in the style not
   being used.
 
 - Fixed an issue in :doc:`google-readability-avoid-underscore-in-googletest-name
   <clang-tidy/checks/google/readability-avoid-underscore-in-googletest-name>` when using
   ``DISABLED_`` in the test suite name.
 
 - Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  <clang-tidy/checks/modernize/concat-nested-namespaces>` when using macro between 
+  <clang-tidy/checks/modernize/concat-nested-namespaces>` when using macro between
   namespace declarations could result incorrect fix.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move
   <clang-tidy/checks/performance/no-automatic-move>` when warning would be
   emitted for a const local variable to which NRVO is applied.
 
 Removed checks
 ^^^^^^^^^^^^^^

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 dummyBeforeBegin;extern "C" int externX;int dummyAfterEnd;
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: redundant 'externX' declaration [readability-redundant-declaration]
+// CHECK-FIXES: {{^}}int dummyBeforeBegin;int dummyAfterEnd;{{$}}