fix(3.12_generics_syntax): Fixed formatting of 3.12 generics syntax

[Exgene]

Fix type parameter expansion when function parameters have trailing comma

Fixes #4740

Problem

When a function with type parameters has a trailing comma in its parameter list, Black incorrectly expanded the type parameter brackets instead of the function parameter list:

Before:

def func1[T: (int, str)](a,): ...

# Formatted to (incorrect):
def func1[
    T: (int, str)
](a,): ...

After:

def func1[T: (int, str)](a,): ...

# Formats to (correct):
def func1[T: (int, str)](
    a,
): ...

Root Cause

The issue was in left_hand_split() which processes both LSQB (square brackets for type parameters) and LPAR (parentheses for function parameters) as potential split points. When a trailing comma forced a split, it would sometimes choose the type parameter brackets instead of the function parameter parentheses.

Solution

Modified left_hand_split() to avoid selecting parentheses ( as split points when they occur inside type parameter brackets [...]. This ensures type parameters stay on the head line and the split happens at the function parameter list where the trailing comma actually exists.

Closes #4740

I need some reference on how these test cases work so if you could help me with that it would be amazing and ill add the relevant test cases for it

[tusharsadhwani]

Use this contributing basics section to setup the project: https://black.readthedocs.io/en/latest/contributing/the_basics.html#technicalities

and then run pytest.

For adding the new tests, follow the "Testing" section on the same page.

[tusharsadhwani]

Also please add a simple explanation entry in the changelog, take a peek at merged PRs to get an idea.

[Exgene]

Use this contributing basics section to setup the project: https://black.readthedocs.io/en/latest/contributing/the_basics.html#technicalities

and then run pytest.

For adding the new tests, follow the "Testing" section on the same page.

Oh no i have done the testing setup and ran the tests no regressions seem to be found, I just didn't understand how the test cases quite worked so i wanted references on that to add for this particular case

[Exgene]

Also please add a simple explanation entry in the changelog, take a peek at merged PRs to get an idea.

Yeah sure on it, Thanks

[tusharsadhwani]

From the doc:

Normally, tests should be created as files in the tests/data/cases directory. These files consist of up to three parts:
[...]

Create a new file to create a new test. You can start with just the unformatted and formatted code snippet from your PR description, and then expand from there to cover more cases.

[Exgene]

From the doc:

Normally, tests should be created as files in the tests/data/cases directory. These files consist of up to three parts:
[...]

Create a new file to create a new test. You can start with just the unformatted and formatted code snippet from your PR description, and then expand from there to cover more cases.

Yeah i got it, Didn't really see the section which explains how it works, Also i don't think this minor change requires an entry in the changes.md?

[tusharsadhwani]

If that docs section felt like it's lacking context you can edit and improve it in another PR, happy to review.

Any change that has a user visible effect is a significant enough change to warrant a changelog entry IMO.

[Exgene]

If that docs section felt like it's lacking context you can edit and improve it in another PR, happy to review.

Any change that has a user visible effect is a significant enough change to warrant a changelog entry IMO.

Yes added basic test cases and updated the changelog to reflect the issue to be fixed should be okay now, Thanks!

[tusharsadhwani]

Make sure not to format the test file, you seem to have changed the test file more than you intended to.

Also you should add a few more complex test cases that are adjacent to this bug, to ensure we handle those well too and don't regress in the future. Single or many item tuples for example, and maybe nested tuples.

[Exgene]

Make sure not to format the test file, you seem to have changed the test file more than you intended to.

Also you should add a few more complex test cases that are adjacent to this bug, to ensure we handle those well too and don't regress in the future. Single or many item tuples for example, and maybe nested tuples.

Oh right im dumb i ran the formatter on the test files, Will revert and add more test cases and try them

[Exgene]

Updated with multiple tests as well and made the required changes, Let me know if any other changes are required

[MeGaGiGaGon]

Thanks for the PR, I'm surprised it was that simple of a change. One more thing before this is good to go, since my previous fix for this already fixed the crash, and this code bug does not cause a crash (as far as I can tell), this will have to go in preview to prevent changing the stable style. #4764 is an example of how to do that, but the basic process is:

1. Add a new enum variant to Preview in mode.py
2. Use if Preview.<variant name> in self.mode to gate the new behavior behind preview
3. Move tests to a new preview specific file starting with # flags: --preview
4. Update future_style.md with a short description of the new mode changes
5. Update black.schema.json with the new preview name

Sorry that it's a bit of a headache to do, but we've recently been a bit loose with the stability policy, and I've been trying to make sure we follow it more closely.

For a bit more info, here's a link to the stability policy.

[Exgene]

Thanks for the PR, I'm surprised it was that simple of a change. One more thing before this is good to go, since my previous fix for this already fixed the crash, and this code bug does not cause a crash (as far as I can tell), this will have to go in preview to prevent changing the stable style. #4764 is an example of how to do that, but the basic process is:

1. Add a new enum variant to Preview in mode.py
2. Use if Preview.<variant name> in self.mode to gate the new behavior behind preview
3. Move tests to a new preview specific file starting with # flags: --preview
4. Update future_style.md with a short description of the new mode changes
5. Update black.schema.json with the new preview name

Sorry that it's a bit of a headache to do, but we've recently been a bit loose with the stability policy, and I've been trying to make sure we follow it more closely.

For a bit more info, here's a link to the stability policy.

Yeah no problem thats honestly fair, Ill do that thanks!

[Exgene]

Thanks for the PR, I'm surprised it was that simple of a change. One more thing before this is good to go, since my previous fix for this already fixed the crash, and this code bug does not cause a crash (as far as I can tell), this will have to go in preview to prevent changing the stable style. #4764 is an example of how to do that, but the basic process is:

1. Add a new enum variant to `Preview` in `mode.py`

2. Use `if Preview.<variant name> in self.mode` to gate the new behavior behind preview

3. Move tests to a new preview specific file starting with `# flags: --preview`

4. Update `future_style.md` with a short description of the new mode changes

5. Update `black.schema.json` with the new preview name

Sorry that it's a bit of a headache to do, but we've recently been a bit loose with the stability policy, and I've been trying to make sure we follow it more closely.

For a bit more info, here's a link to the stability policy.

How do i gate this cause i can't really access the self.mode in linegen.py where the only change is outside the class i think?

[Exgene]

@MeGaGiGaGon Let me know if this works

[Exgene]

@MeGaGiGaGon Yes done

[Exgene]

Oh right it won't working on python version below 3.12 right i should update that for the test cases