LGTM.
Any ideas for improvement, @JonasToth?
I'd rather have it merged and improve on it later if there are ideas on how to do it better.

A few more ideas for enhancements.

LGTM. But I'm not a code owner here and I don't know if you need an acceptance of one of them.
Great job.

Some more minor remarks. I'll give this check a try to see how it behaves on some of my projects.
I agree that a high rate of false positives is possible (and is a bit of spoiler) but I wouldn't reject this IMO useful check because of that.
Anyway, everything looks pretty clean for me.

Just a few minor remarks and a possible workaround for testing CHECK-FIXES: [[nodiscard]].

Fixed diff.

Applied changes as per comments.

Actually, I can make it an option for this check to skip or not constexpr ifs, WDYT?

In D53372#1267728, @lebedev.ri wrote:

I think it would be good to add some more explanation as to *why* that else has to be kept.

Thanks for the changes. LGTM.

Any thoughts on this?

Could you add just one more test for PAS_Middle please? A single line will be just enough.
Otherwise, LGTM.

That's precisely what I've written, but, as I'd said before, such tests pass already without any modification in NamespaceEndCommentsFixer.

This was fixed by https://reviews.llvm.org/rL312904 and other commits.

I confirm what I observed before: when invoking tests in unittests/Format/NamespaceEndCommentsFixerTest.cpp, the const AnnotatedLine *line parameter in getNamespaceToken gets one big line that includes both namespace and { (something like namespace\n{\n..., whereas in tests invoked from unittests/Format/FormatTests.cpp, there is a separate line with namespace\n and another one with {\n.

Ok. Nice patch. You can close https://bugs.llvm.org/show_bug.cgi?id=19986 now.

Minor: use FormatToken::isNot instead of !FormatToken::is.

There is one big missing thing here: PointerAlignment. Actually only PAS_Left is taken into account.
There are 3 possible options:
Left: auto& [a, b] = f();
Middle: auto & [a, b] = f();
Right: auto &[a, b] = f();

In D37904#873860, @krasimir wrote:

Could you please move the test that adds a namespace end comment to unittests/Format/NamespaceEndCommentsFixerTest.cpp?

Superseded by rL312125.

Fix: use do-while loop.

Extract method.

Fix line endings again.

Revert unintended format changes.

Fix line endings.

Thanks Daniel.

Fix omitted dereference.

In D30567#708797, @alexfh wrote:

Do you have commit rights?

Trim spaces only everywhere. Fix test.

Hi Alex, is it OK now?

Trim only spaces.

Hi Alex and sorry for the late reply.

Remove blank line.

For the moment, just a few nitty-gritty comments inline.
What I miss here is (as already pointed by someone) an example on how to write a new module, register it etc.

Nice job! Thanks for taking my remarks into account.

Some minor remarks. Sorry for being finicky :).

I'm really interested in the manner this check works when a typedef has multiple declarations in it (same example as in the comment):

typedef int m_int, *m_int_p, &m_int_r, m_int_arr[10], (&m_int_fun)(int, int);

Minor remark.

Ping?

Applied comments.

• AllowShortFunctionsOnASingleLine: true didn't work with BreakBeforeBraces: Linux/Allman.
• Add test checking that non-empty functions in styles with BraceWrapping.AfterFunction = true don't get merged into one line. Fix the merge condition.

Where is the code in the CL that handles extracting that value from a JSON string? Because it looks like you're just building an array list of the trivial non-JSON style names, so why couldn't that be an enum? I don't know mucha bout clang-format, but looking at the comments it seems like the JSON only comes into play when you're reading a .clang-format file, and in that case the value of the enum would be Style.File (until you finish reading that, at which case you set it to Style.LLVM etc).

The Style option is actually passed to clang-format -style parameter and it's clang-format that parses it. It can be a predefined type (one of: 'none', 'file', 'LLVM', etc.) or a JSON (or JSON-like?) string.

Hi Zachary, just to answer your comments. I have done it on purpose not to use enum, because clang-format style can be actually a JSON string, e.g. {BasedOnStyle: "LLVM", IndentWidth: 4}, so it wouldn't translate into an enum (to my knowledge at least). Besides, I would like to have the possibility not to add a new enum value if there is a new style in clang-format.
Please correct me if I'm wrong about enums.

Ping?

Applied comments and done some minor clean up.

Applied Aaron's comments. Removed unused using.

Assume-Filename option will now give an error when there are quotes in the filename.

Add option converters for filenames (prohibiting quotes) and styles (giving a list of predefined styles).

Applied suggestions from comments.

Add FIXME comment.

Escape only '<' and '&'.
Remove unrelated changes from the revision.

Fix description formatting.

Escape XML-reserved characters.

Out of date.

Can you land it please?
I don't have write access.

Corrected in http://reviews.llvm.org/rL246582.