Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Initial version of python try with all things #1704

Merged
merged 16 commits into from
Sep 26, 2024
Merged

Initial version of python try with all things #1704

merged 16 commits into from
Sep 26, 2024

Conversation

KuechA
Copy link
Contributor

@KuechA KuechA commented Sep 20, 2024 •
edited
Loading

To add support for python's try statements in accordance with outcome of the recent discussion, this PR

  • Adds an elseBlock to the TryStatement
  • Handles the Python.AST.Try node

Requires @konradweiss to add the elseBlock in the EvaluationOrderGraphPass.

@KuechA KuechA mentioned this pull request Sep 20, 2024
Closed
@oxisto oxisto changed the title Initial version of python try with all things Initial version of python try with all things Sep 23, 2024
lshala
lshala reviewed Sep 23, 2024
View reviewed changes
Copy link
Contributor

@lshala lshala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the changes and they look good to me. However, since I don't have much experience yet, I'm uncertain about clicking on 'approve'.

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/TryStatement.kt Show resolved Hide resolved
@konradweiss konradweiss self-assigned this Sep 23, 2024
@konradweiss konradweiss mentioned this pull request Sep 23, 2024
Open
@oxisto
Copy link
Member

oxisto commented Sep 23, 2024

@konradweiss needs to implement the EOG

@konradweiss
Copy link
Collaborator

Currently working on the tests for the EOG handling

@konradweiss konradweiss marked this pull request as ready for review September 24, 2024 18:35
maximiliankaul
maximiliankaul approved these changes Sep 25, 2024
View reviewed changes
Copy link
Contributor

@maximiliankaul maximiliankaul left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I'd appreciate to split this into multiple PRs all doing one thing in the future.

...language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/StatementHandler.kt Outdated Show resolved Hide resolved
oxisto
oxisto requested changes Sep 25, 2024
View reviewed changes
Copy link
Member

@oxisto oxisto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

taking back my approve because of the issue with anonymous identifiers

@oxisto oxisto enabled auto-merge (squash) September 26, 2024 12:08
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Sep 26, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@oxisto oxisto merged commit 81a82ac into main Sep 26, 2024
3 checks passed
@oxisto oxisto deleted the ak/pythonOrElse branch September 26, 2024 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lshala lshala lshala left review comments

@konradweiss konradweiss konradweiss left review comments

@maximiliankaul maximiliankaul maximiliankaul approved these changes

@oxisto oxisto oxisto approved these changes

Assignees

@konradweiss konradweiss

Labels
python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@KuechA @oxisto @konradweiss @maximiliankaul @lshala