[clang-tools-extra] f15b786 - [clang-tidy] [clangd] Avoid multi-line diagnostic range for else-after-return diagnostic

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-11-29T18:32:23-05:00
New Revision: f15b7869e5afbd6c24ef440b0b62593e80fbd24f

URL: https://github.com/llvm/llvm-project/commit/f15b7869e5afbd6c24ef440b0b62593e80fbd24f
DIFF: https://github.com/llvm/llvm-project/commit/f15b7869e5afbd6c24ef440b0b62593e80fbd24f.diff

LOG: [clang-tidy] [clangd] Avoid multi-line diagnostic range for else-after-return diagnostic

Fixes https://bugs.llvm.org/show_bug.cgi?id=47809

Differential Revision: https://reviews.llvm.org/D92272

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
    clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
index 79e3cded45bc..89bb02e78cc6 100644
--- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -267,7 +267,8 @@ void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
       // If the if statement is the last statement its enclosing statements
       // scope, we can pull the decl out of the if statement.
       DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-                               << ControlFlowInterruptor;
+                               << ControlFlowInterruptor
+                               << SourceRange(ElseLoc);
       if (checkInitDeclUsageInElse(If) != nullptr) {
         Diag << tooling::fixit::createReplacement(
                     SourceRange(If->getIfLoc()),
@@ -302,7 +303,8 @@ void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
       // If the if statement is the last statement its enclosing statements
       // scope, we can pull the decl out of the if statement.
       DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-                               << ControlFlowInterruptor;
+                               << ControlFlowInterruptor
+                               << SourceRange(ElseLoc);
       Diag << tooling::fixit::createReplacement(
                   SourceRange(If->getIfLoc()),
                   (tooling::fixit::getText(*If->getInit(), *Result.Context) +
@@ -319,7 +321,7 @@ void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
   }
 
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
-                           << ControlFlowInterruptor;
+                           << ControlFlowInterruptor << SourceRange(ElseLoc);
   removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc);
 }
 

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index ba7029e54dbb..c6a14aeeb469 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -474,6 +474,24 @@ TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
 }
 
+TEST(DiagnosticTest, ElseAfterReturnRange) {
+  Annotations Main(R"cpp(
+    int foo(int cond) {
+    if (cond == 1) {
+      return 42;
+    } [[else]] if (cond == 2) {
+      return 43;
+    }
+    return 44;
+    }
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
+  EXPECT_THAT(
+      TU.build().getDiagnostics(),
+      ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'")));
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that: