[clang-format] Add qualifiers (actual) alignments

[KBoumghar]

There's currently a QualifierAlignment option in ClangFormat, although I'd argue it is misnamed ; it doesn't align anything, it simply order qualifiers.

Currently, I believe that if you write :

int a = 0;
const int b = 0;

there is no way to obtain :

      int a = 0;
const int b = 0;

This could be added to the options of AlignConsecutiveAssignements and AlignConsecutiveDeclarations, although it'd also be useful for parameters as well, i.e. :

void foo(const type1 looooooooooooooooooooongarg1,
         type2       arg2){}

becomes :

void foo(const type1 looooooooooooooooooooongarg1,
               type2 arg2){}

[llvmbot]

@llvm/issue-subscribers-clang-format

Author: None (KBoumghar)

There's currently a `QualifierAlignment` option in ClangFormat, although I'd argue it is misnamed ; it doesn't align anything, it simply order qualifiers.

Currently, I believe that if you write :

int a = 0;
const int b = 0;

there is no way to obtain :

      int a = 0;
const int b = 0;

This could be added to the options of AlignConsecutiveAssignements and AlignConsecutiveDeclarations, although it'd also be useful for parameters as well, i.e. :

void foo(const type1 looooooooooooooooooooongarg1,
         type2       arg2){}

becomes :

void foo(const type1 looooooooooooooooooooongarg1,
               type2 arg2){}

[mydeveloperday]

You QualifierAlignment Left/Right/Custom, the QualifierOrder lets you control the order... "naming is hard" but its only confusing for 5 minutes.

[mydeveloperday]

I agree this is an addition to AlignConsecutiveAssignements and AlignConsecutiveDeclarations maybe to suport Left/Right not just true/false

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: None (KBoumghar)

There's currently a `QualifierAlignment` option in ClangFormat, although I'd argue it is misnamed ; it doesn't align anything, it simply order qualifiers.

Currently, I believe that if you write :

int a = 0;
const int b = 0;

there is no way to obtain :

      int a = 0;
const int b = 0;

This could be added to the options of AlignConsecutiveAssignements and AlignConsecutiveDeclarations, although it'd also be useful for parameters as well, i.e. :

void foo(const type1 looooooooooooooooooooongarg1,
         type2       arg2){}

becomes :

void foo(const type1 looooooooooooooooooooongarg1,
               type2 arg2){}

[KBoumghar]

@mydeveloperday I was thinking of maybe adding a bool AlignQualifiers, although I'm not sure how it'd work for different qualifiers (maybe bool AlignType?), so your idea of Left/Right might be better. If this works for you (and you still believe it is a good first-time issue), I'd like to try my hand at it if I can get it assigned to me.

Any opinion on the parameter part (or does this work with declaration? I'm not sure...), this is the one my team is bugging me the most about.

[AlyElashram]

This has been stale for a while , if it's a good first issue i can take a look at it. @mydeveloperday . Please assign it to me.

[owenca]

It doesn't look like a "good first issue" to me.

[AlyElashram]

I'm still interested to take a look as I would like to actually use this type of formatting.
@owenca

[AlyElashram]

Took me a while to understand the current behavior , but I have a couple of questions.

Are these 2 scenarios correct for AlignConsecutiveAssignments
Case 1 :

int a = 1;
const double b = 2;

should this be formatted where the i from int aligns with the d from double ? and then align the equal signs together.

in this way the Assignment operator still aligns and the types are aligned together. If this is the approach then we should only consider the const qualifier width and just align with it.

Another case I'd like to consider is the case of pointers. For example:

int *a = nullptr;
const double* b = nullptr;

Again would we align the i of int with the d from double ?

And then for AlignConsecutiveDeclarations we would align the variable names to be aligned together as well. Are my assumptions correct for both ?

@KBoumghar @mydeveloperday

[AlyElashram]

Just posting my findings here before unassigning this as it's a bit complicated to tackle from someone who's new to this Repo.

In AlignConsecutiveAssignments the function that checks for the actual anchor to align is a lambda function passed to AlignTokens, in AlignConsecutiveAssignments this function checks for a match with the "=" sign , this behavior differs in AlignConsecutiveDeclarations as it searches for declarations not the equal sign. Both of these functions live inside the WhitespaceManager.

The suggested change would be to either call AlignTokens twice inside each function , once to align the qualifiers/data types and the other to align the declarations/assignments. (This is a suggestion not a proven working concept).

Other areas that require change would be Format.h and Format.cpp where you would either pass an extra variable to align qualifiers right or change the existing behaviour for AlignConsecutiveAsssignments and AlignConsecutiveDeclarations to support left and right. The suggested change would live inside MappingTraits<FormatStyle::AlignConsecutiveStyle>.