[clang-tidy] fix concat-nest-namespace fix hint remove the macro

[HerrCai0907]

Summary

Partially fixed #60035
This patch refactor the FixHint for concat-nest-namespace.

1. remove each namespace except the last non-nest namespace.
2. replace the last non-nest namespace with the new name.

It can remain the comment / pragma / macro between namespace and update the close comment.

[HerrCai0907]

clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp

72

This part of code copy from Lexer::findNextToken except this line. Should I refactor Lexer::findNextToken for example add a parameter to control it?
But Lexer::findNextToken is involved with lots of code. Maybe use another patch to do it?

[PiotrZSL]

You changed half of this check logic, please take a look into those issues for this check:
https://github.com/llvm/llvm-project/issues/60051
https://github.com/llvm/llvm-project/issues/60048
https://github.com/llvm/llvm-project/issues/60035
https://github.com/llvm/llvm-project/issues/57530
https://github.com/llvm/llvm-project/issues/56022
https://github.com/llvm/llvm-project/issues/43351
https://github.com/llvm/llvm-project/issues/41015
Add issues that you fixing to commit message.

Also look into these patches:
https://reviews.llvm.org/D61989
https://reviews.llvm.org/D141787

Update release notes (add info about fixes for this check).

Would be good to run it on some code to check how this behave, except that looks fine.

clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp

51–52

check if you can do something with this...

add test for:

with something like:

namespace x1 {
// namespace x3::x4 {
namespace x2 {
void foo();
}
// }
}

72

Dont change Lexer.

Look into clang-tools-extra/clang-tidy/utils/LexerUtils.h, if you can find something useful there,
if not then move this function there.

clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h

8

is check also fixes namespace comments ?

[PiotrZSL]

clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h

8

ok, neverming... lets let other check fix them...

[HerrCai0907]

clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp

51–52

It still have false negative.
But this should be done in another PR. In here I just extract a common function without changing the logic.

[HerrCai0907]

refactor some code and add release note

[PiotrZSL]

clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp

71

Remove this debug.

90

what if it is //namespace ? or /* namespace XYZ */

129

remove redundant {} from this function, they not needed for single line statements.

clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp

224

add same test but without spaces and move //CHECK-MESSAGES outside namespace.

[HerrCai0907]

add feature update close comment in fixhint
update acc. comment

[PiotrZSL]

Overall looks +- fine, these are last comments from me.

clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp

90

ok, but it still can be things like // namespace XYZ
So instead adding single space, consider using some trim/strip.
but thats minor issues, side effect would be just an leftover namespace comment.
add some test with something like // namespace XYZ - closing namespace to verify that some custom comments wont be removed, and make sure that documentation mention this, that we remove only namespace comments that follow some known standard.

you could also check for:

namespace C
{
} // C

129

this index looks reundant, you could use range-for

146–155

consider ordering removals from last to first...
personally i sometimes run into issues when we first removed something before last change, but that could be already fixed, or we may not run into this issue in this check

[HerrCai0907]

update according to comment

[HerrCai0907]

reorder fixhint

[HerrCai0907]

clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp

90

I think it can cover most of scenarios. It don't worth to introduce a mechanism to archive it.
Because something false positive like // name space XYZ should also be considered.