Skip to content

Deprecate a parameter name that is not a Python identifier - #3866

Open
kdeldycke wants to merge 1 commit into
pallets:stablefrom
kdeldycke:deprecate-non-identifier-names
Open

kdeldycke wants to merge 1 commit into
pallets:stablefrom
kdeldycke:deprecate-non-identifier-names

Conversation

@kdeldycke

@kdeldycke kdeldycke commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

This PR prepares the ground for #3827, which is going to refuse parameter names that are not valid Python identifiers.

Of course, I fucked-up the same way I fucked-up 2 months ago with #3701 , when juggling between main and stable branches made me push to stable. 😫

So:

  • I pushed by mistake e2bddbc
  • I reverted my mistake on stable with d036881
  • And this PR is what it should have been all along

Now, for the PR itself: this contain additional DeprecationWarning that detect when Parameter, Argument or Option gets a name that will be not allowed in the future 9.0.0 release. I.e. when these parameters are not Python identifier according to str.identifier.

This PR also copy the majority of tests that were originally proposed in #3827, so the behavior of the name is locked down and frozen. This will prevent tweaking or introducing regressions during the full 8.x cycle.

@kdeldycke kdeldycke added the parsing Parsing, parameters, commands, chaining, context label Sep 11, 2026
@kdeldycke kdeldycke added this to the 8.5.1 milestone Sep 11, 2026
@kdeldycke kdeldycke added the tests Click's own test suite and CI workflows label Sep 11, 2026
@kdeldycke
kdeldycke force-pushed the deprecate-non-identifier-names branch from 62cff1d to 3c5be86 Compare September 11, 2026 13:35
@davidism

Copy link
Copy Markdown
Member

What do you think, should we also check keyword.iskeyword? Might be good to warn about it and see if anything comes up.

@kdeldycke

Copy link
Copy Markdown
Collaborator Author

What do you think, should we also check keyword.iskeyword? Might be good to warn about it and see if anything comes up.

Ah. That might be a good idea: test drive that check during a long 8.x cycle and see if it show up... If I implement this, what kind of severity should it triggers? A DeprecationWarning like non-Python-IDs ?

@davidism

Copy link
Copy Markdown
Member

Deprecation seems fine. Like I said in the other thread, it could be more disruptive, but it also seems to match the intention of the str.isidentifier check.

Prepares the ground for pallets#3827 and future 9.0.0 release
@kdeldycke
kdeldycke force-pushed the deprecate-non-identifier-names branch from 3c5be86 to eb37bea Compare September 11, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parsing Parsing, parameters, commands, chaining, context tests Click's own test suite and CI workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants