Skip to content

Commit

Permalink
feat(python): encode positional-only arguments in signatures
Browse files Browse the repository at this point in the history
Now that Python 3.7 reached end-of-life, and since Python 3.8 and
greater have support for positional-only argument notation (any argument
before a `/` delimiter may only be passed positionally), adjust code
generation to leverage this feature in order to address issues where
inheritance hierarchies would rename parameters, which is a non-issue in
all languages but Python, where those could always be provided as
keyword arguments before.

Fixes #2927

BREAKING CHANGE: the generated Python code now requires Python 3.8 or
later and encodes positional arguments as positional-only, making
keyword-style usage impossible. Users who used the keyword-style
convention need to update their code to use the positional syntax
instead.
  • Loading branch information
RomainMuller committed Jul 26, 2023
1 parent ef6e5b1 commit 9e03b46
Show file tree
Hide file tree
Showing 5 changed files with 213 additions and 199 deletions.
24 changes: 19 additions & 5 deletions packages/jsii-pacmak/lib/targets/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,9 +639,13 @@ abstract class BaseMethod implements PythonBase {
const liftedProperties = this.getLiftedProperties(context.resolver);

if (liftedProperties.length >= 1) {
// Anything before the 1st keyword argument is positional-only
if (pythonParams.length > 0) {
pythonParams.push('/'); // marks the end of positional-only arguments
}
// All of these parameters are keyword only arguments, so we'll mark them
// as such.
pythonParams.push('*');
pythonParams.push('*'); // marks the start of keyword-only arguments

// Iterate over all of our props, and reflect them into our params.
for (const prop of liftedProperties) {
Expand Down Expand Up @@ -2102,7 +2106,7 @@ class Package {
package_dir: { '': 'src' },
packages: modules.map((m) => m.pythonName),
package_data: packageData,
python_requires: '~=3.7',
python_requires: '~=3.8',
install_requires: [
`jsii${toPythonVersionRange(`^${VERSION}`)}`,
'publication>=0.0.3',
Expand All @@ -2115,7 +2119,6 @@ class Package {
'Operating System :: OS Independent',
'Programming Language :: JavaScript',
'Programming Language :: Python :: 3 :: Only',
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
Expand Down Expand Up @@ -2236,7 +2239,7 @@ class Package {
code.line();
code.line('[tool.pyright]');
code.line('defineConstant = { DEBUG = true }');
code.line('pythonVersion = "3.7"');
code.line('pythonVersion = "3.8"');
code.line('pythonPlatform = "All"');
code.line('reportSelfClsParameterName = false');
code.closeFile('pyproject.toml');
Expand Down Expand Up @@ -3144,6 +3147,8 @@ function emitParameterTypeChecks(
const [name] = param.split(/\s*[:=#]\s*/, 1);
if (name === '*') {
return { kwargsMark: true };
} else if (name === '/') {
return { positionalOnlyEndMark: true };
} else if (name.startsWith('*')) {
return { name: name.slice(1), is_rest: true };
}
Expand All @@ -3156,7 +3161,12 @@ function emitParameterTypeChecks(
const typesVar = slugifyAsNeeded('type_hints', paramNames);

let openedBlock = false;
for (const { is_rest, kwargsMark, name } of paramInfo) {
for (const {
is_rest,
kwargsMark,
positionalOnlyEndMark,
name,
} of paramInfo) {
if (kwargsMark) {
if (!context.runtimeTypeCheckKwargs) {
// This is the keyword-args separator, we won't check keyword arguments here because the kwargs will be rolled
Expand All @@ -3166,6 +3176,10 @@ function emitParameterTypeChecks(
// Skip this (there is nothing to be checked as this is just a marker...)
continue;
}
if (positionalOnlyEndMark) {
// This is the end-of-positional-only-arguments separator, this is just a marker.
continue;
}

if (!openedBlock) {
code.openBlock('if __debug__');
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 9e03b46

Please sign in to comment.