fix(python): self as property name leads to assignment error

[skorfmann]

When a property with the name self exists in a class, it leads to
a runtime error. This slugifies the conventional self parameter
as needed to avoid this problem without compromising on the
developer experience.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[mergify]

The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged.
[Conventional Commits]: https://www.conventionalcommits.org

[RomainMuller]

Thanks for this contribution!

This is currently missing a test that demonstrates the issue is fixed.

I think right now this will cause a warning to be emitted, but it will not fix code generation so the name is correctly slugified.

[skorfmann]

This is currently missing a test that demonstrates the issue is fixed.

Could you point me to a similar test?

I think right now this will cause a warning to be emitted, but it will not fix code generation so the name is correctly slugified.

Yes, I think so too. What would be the correct way to slugify this?

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
• Commit ID: d8503b2
• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[RomainMuller]

With respects to the slugification, there appears to be a duplicated reserved words list in jsii-pacmak that fails to include self (bonus points if you find a way to de-duplicate, but I'm cool if you don't want to bother with that, too):

jsii/packages/jsii-pacmak/lib/targets/python.ts

Lines 71 to 76 in 1ff2557

	const PYTHON_KEYWORDS = [
	'False', 'None', 'True', 'and', 'as', 'assert', 'async', 'await', 'break', 'class',
	'continue', 'def', 'del', 'elif', 'else', 'except', 'finally', 'for', 'from',
	'global', 'if', 'import', 'in', 'is', 'lambda', 'nonlocal', 'not', 'or', 'pass',
	'raise', 'return', 'try', 'while', 'with', 'yield'
	];

---

With respects to the tests... This is going to involve:

1. Adding stuff in jsii-calc to reproduce the issue you've identified:
   

   jsii/packages/jsii-calc/lib/compliance.ts

   Lines 896 to 961 in 1ff2557

   	export class PythonReservedWords {
   	public and() {}
   	public as() {}
   	public assert() {}
   	public async() {}
   	public await() {}
   	public break() {}
   	public class() {}
   	public continue() {}
   	public def() {}
   	public del() {}
   	public elif() {}
   	public else() {}
   	public except() {}
   	public finally() {}
   	public for() {}
   	public from() {}
   	public global() {}
   	public if() {}
   	public import() {}
   	public in() {}
   	public is() {}
   	public lambda() {}
   	public nonlocal() {}
   	public not() {}
   	public or() {}
   	public pass() {}
   	public raise() {}
   	public return() {}
   	public try() {}
   	public while() {}
   	public with() {}
   	public yield() {}
   	}

2. Adding a new test in the Python runtime's test suite that "runs into the bug" (which would pass with the fix in, so you know you've nailed it for good!)
   

   jsii/packages/@jsii/python-runtime/tests/test_compliance.py

   Lines 1168 to 1170 in 1ff2557

   	def test_collection_of_interfaces_map_of_interfaces():
   	for elt in InterfaceCollections.map_of_interfaces().values():
   	assert getattr(elt, "ring") is not None

In any case - we'll happily provide help if you don't know how to proceed!

[RomainMuller]

Reading up a bit, it appears that self is technically not a keyword in Python, so we're abusing a little here if we add it to the PYTHON_KEYWORDS list in jsii-pacmak (it's actually "fine" to have a warning about it in jsii, still).

I reckon the real fix is going to possibly be with code-generation of instance methods in jsii-pacmak (the targets/python.ts file is where it's at).

I can let you have a look here and see if you find your way around what needs to change... And then I can have a look, too, if you get nowhere or can't afford to spend that kind of time on this :)

[skorfmann]

Cool, thanks for providing me the context. I'll take a look at it.

[RomainMuller]

I have improved the solution here, but realized the current Submodules feature of jsii generates invalid Python code for the test I would like to introduce (making it impossible to load).

This would need fixing before the compliance test can be added, however we could ship as-is since the original bug is likely addressed. I'd still kind of prefer being able to add the next test however (because I'm not sure the instance property/method name conflict results in code that'll behave as intended).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
• Commit ID: 50e3f84
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
• Commit ID: 098c0bb
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
• Commit ID: cbff2d1
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Pull request has been modified.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
• Commit ID: b686c68
• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
• Commit ID: c3935cb
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

[mergify]

Merging (with squash)...