[clang-tidy] Adjust location of namespace comment diagnostic

[ilya-biryukov]

Summary

If there is no comment, place it at the closing brace of a namespace
definition. Previously it was placed at the next character after the
closing brace.

The new position produces a better location for highlighting in clangd
and does not seem to make matters worse for clang-tidy.

[alexfh]

LG. Thanks!

clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp

 //===--- NamespaceCommentCheck.cpp - clang-tidy ---------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "NamespaceCommentCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringExtras.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace readability {
if (Tok.is(tok::comment) && NextTokenIsOnSameLine) {
     // multi-line or there may be other tokens behind it.
   }
 
   std::string NamespaceName =
       ND->isAnonymousNamespace()
           ? "anonymous namespace"
           : ("namespace '" + NestedNamespaceName.str() + "'");
 
-  diag(AfterRBrace, Message)
+  // Place diagnostic at an old comment, or closing brace if we did not have it.
+  SourceLocation DiagLoc =
+      OldCommentRange.getBegin() != OldCommentRange.getEnd()
+          ? OldCommentRange.getBegin()
+          : ND->getRBraceLoc();
+
+  diag(DiagLoc, Message)
       << NamespaceName
       << FixItHint::CreateReplacement(
              CharSourceRange::getCharRange(OldCommentRange),
              std::string(SpacesBeforeComments, ' ') +
                  (IsNested
                       ? getNamespaceComment(NestedNamespaceName, NeedLineBreak)
                       : getNamespaceComment(ND, NeedLineBreak)));
   diag(ND->getLocation(), "%0 starts here", DiagnosticIDs::Note)
       << NamespaceName;
 }
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp

 // RUN: %check_clang_tidy %s google-readability-namespace-comments %t
 
 namespace n1 {
 namespace n2 {
 
 
 void f(); // So that the namespace isn't empty.
 
 
-// CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n2' not terminated with a closing comment [google-readability-namespace-comments]
+// CHECK-MESSAGES: :[[@LINE+4]]:1: warning: namespace 'n2' not terminated with a closing comment [google-readability-namespace-comments]
 // CHECK-MESSAGES: :[[@LINE-7]]:11: note: namespace 'n2' starts here
-// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1' not terminated with
+// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'n1' not terminated with
 // CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'n1' starts here
 }}
 // CHECK-FIXES: }  // namespace n2
 // CHECK-FIXES: }  // namespace n1
 
 #define MACRO macro_expansion
 namespace MACRO {
 void f(); // So that the namespace isn't empty.
 // 1
 // 2
 // 3
 // 4
 // 5
 // 6
 // 7
-// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'macro_expansion' not terminated with
+// CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'macro_expansion' not terminated with
 // CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'macro_expansion' starts here
 }
 // CHECK-FIXES: }  // namespace macro_expansion
 
 namespace short1 {
 namespace short2 {
 // Namespaces covering 10 lines or fewer are exempt from this rule.
 

clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp

 // RUN: %check_clang_tidy %s google-readability-namespace-comments %t
 
 namespace n1::n2 {
 namespace n3 {
 
 // So that namespace is not empty.
 void f();
 
 
-// CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n3' not terminated with
+// CHECK-MESSAGES: :[[@LINE+4]]:1: warning: namespace 'n3' not terminated with
 // CHECK-MESSAGES: :[[@LINE-7]]:11: note: namespace 'n3' starts here
-// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1::n2' not terminated with a closing comment [google-readability-namespace-comments]
+// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'n1::n2' not terminated with a closing comment [google-readability-namespace-comments]
 // CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'n1::n2' starts here
 }}
 // CHECK-FIXES: }  // namespace n3
 // CHECK-FIXES: }  // namespace n1::n2
 

clang-tools-extra/trunk/test/clang-tidy/select-checks.cpp

 // RUN: clang-tidy %s -checks='-*,llvm-namespace-*' -- 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy %s -checks='-*,an-unknown-check' -- 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK2 %s
 
 // CHECK2: Error: no checks enabled.
 
 namespace i {
 }
-// CHECK: :[[@LINE-1]]:2: warning: namespace 'i' not terminated with a closing comment [llvm-namespace-comment]
+// CHECK: :[[@LINE-1]]:1: warning: namespace 'i' not terminated with a closing comment [llvm-namespace-comment]
 
 // Expect no warnings from the google-explicit-constructor check:
 class A { A(int i); };