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

Implement a throw or raise statement #1733

Merged
merged 34 commits into from
Oct 18, 2024
Merged

Implement a throw or raise statement #1733

merged 34 commits into from
Oct 18, 2024

Conversation

maximiliankaul
Copy link
Contributor

@maximiliankaul maximiliankaul commented Sep 26, 2024 •
edited by KuechA
Loading

This PR implements a new ThrowStatement

  • Check the new node. Does it work for all supported languages?
  • Implement EOG
  • Implement DFG
  • Implement all other relevant passes (are there any?)
  • Implement fluent tests

The language specific implementations will be handled in separate PRs:

  • C++ currently uses a UnaryOperator
  • Implement for Python Python: raise #1741
  • Implement for other languages

@maximiliankaul maximiliankaul self-assigned this Sep 26, 2024
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Sep 27, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
29.6% Coverage on New Code (required ≥ 75%)

See analysis details on SonarCloud

@maximiliankaul maximiliankaul changed the title Add a RaiseStatement Implement a throw or raise statement Sep 27, 2024
@maximiliankaul maximiliankaul removed their assignment Sep 27, 2024
@maximiliankaul maximiliankaul added the graph-changing This change breaks the current graph structure, i.e. it changes semantic of properties and edges label Sep 27, 2024
KuechA
KuechA reviewed Oct 2, 2024
View reviewed changes
cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/edges/flows/DataflowTest.kt Outdated Show resolved Hide resolved
cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/edges/flows/DataflowTest.kt Outdated Show resolved Hide resolved
cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/edges/flows/DataflowTest.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/EvaluationOrderGraphPass.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/DFGPass.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/ThrowStatement.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/ThrowStatement.kt Outdated Show resolved Hide resolved
@KuechA KuechA mentioned this pull request Oct 2, 2024
Merged
maximiliankaul and others added 12 commits October 4, 2024 16:02
@maximiliankaul
copy & paste handleThrowOperator
db77867
@maximiliankaul @KuechA
Update cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/edges/f…
b0ed02b 
…lows/DataflowTest.kt

Co-authored-by: KuechA <[email protected]>
@oxisto @KuechA @maximiliankaul
Rename findSymbols into lookupSymbolByName (#1772)
18afeff 
* Rename `findSymbols` into `lookupSymbolByName`

This PR renames `findSymbols` into `lookupSymbolByName` as a more appropriate name, because it lookups a symbol by its name.

Fixes #1767

* Update cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt

Co-authored-by: KuechA <[email protected]>

* Added documentation

---------

Co-authored-by: KuechA <[email protected]>
@renovate @maximiliankaul
Update dependency rollup to v4.24.0 (#1774)
2c7dd71 
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@oxisto @maximiliankaul
Added language trait HasImplicitReceiver (#1778)
f542681 
Added language trait `HasImplicit Receiver`

This is needed in preparation for #1777 to better handle access to fields/members of a class when an implicit receiver is used.
@oxisto @maximiliankaul
Cleanup of SymbolResolver (#1777)
0187754
@oxisto @maximiliankaul
Fixed crash in getCodeOfSubregion (#1776)
9755f71
@oxisto @maximiliankaul
Add new function lookupUniqueTypeSymbolByName (#1781)
a69f98a 
* Add new function `lookupUniqueTypeSymbolByName`

This adds two new functions `ScopeManager.lookupUniqueTypeSymbolByName` and `Reference.doesReferToType`. This harmonizes a lot of boilerplate code in type resolver, cxx extra pass and resolve ambuigity pass, which were all trying to achieve the same thing.

Fixes #1766

* Fixed issue with Go test, more robust handling of wrapped references

* Addressed code review
@oxisto @maximiliankaul
Make sure to move typeObservers from old to new node when replacing…
b358fa1 
… nodes (#1783)

* Make sure to move `typeObservers` from old to new node when replacing nodes

* Added doc for typeobservers
@oxisto @maximiliankaul
implicit() only triggers code/location update now if its not empty (#…
5e83741 
…1784)

Otherwise, we override the code/location again.
@oxisto @maximiliankaul
Added := as simple operator in Python (#1785)
205a275 
Named expressions in Python use `:=` as operator. Therefore we need to include it in the language definition. Otherwise, the `access` value of a reference will not be set correctly.
@maximiliankaul
code review
89fd8ec
@KuechA
Copy link
Contributor

KuechA commented Oct 9, 2024

We should keep in mind to enable throwing/raising the exception when handling the with statement once this feature is available and #1757 is merged.

@codecov Codecov
Copy link

codecov bot commented Oct 10, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 67.79661% with 19 lines in your changes missing coverage. Please review.

Project coverage is 74.99%. Comparing base (e956427) to head (0be08ca).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...hofer/aisec/cpg/graph/statements/ThrowStatement.kt 48.27% 14 Missing and 1 partial ⚠️
...in/de/fraunhofer/aisec/cpg/graph/builder/Fluent.kt 66.66% 0 Missing and 2 partials ⚠️
.../de/fraunhofer/aisec/cpg/graph/StatementBuilder.kt 83.33% 1 Missing ⚠️
...hofer/aisec/cpg/passes/EvaluationOrderGraphPass.kt 93.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
...n/kotlin/de/fraunhofer/aisec/cpg/passes/DFGPass.kt 82.53% <100.00%> (+0.95%) ⬆️
.../de/fraunhofer/aisec/cpg/graph/StatementBuilder.kt 73.33% <83.33%> (-13.71%) ⬇️
...hofer/aisec/cpg/passes/EvaluationOrderGraphPass.kt 90.79% <93.33%> (+0.15%) ⬆️
...in/de/fraunhofer/aisec/cpg/graph/builder/Fluent.kt 87.21% <66.66%> (-0.26%) ⬇️
...hofer/aisec/cpg/graph/statements/ThrowStatement.kt 48.27% <48.27%> (ø)

... and 119 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maximiliankaul maximiliankaul marked this pull request as ready for review October 10, 2024 13:08
@KuechA
Copy link
Contributor

KuechA commented Oct 10, 2024

The changes itself look ok to me, but the test coverage is a bit low. Easy fixes could be: Try different throws (with and without parentException, with and without exception). It would also make sense to cover the EOG by having some try/catch surrounding the throw

@KuechA KuechA merged commit a12951b into main Oct 18, 2024
4 of 5 checks passed
@KuechA KuechA deleted the mk/raiseStmt branch October 18, 2024 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KuechA KuechA KuechA approved these changes

@oxisto oxisto oxisto approved these changes

@konradweiss konradweiss Awaiting requested review from konradweiss konradweiss is a code owner

Assignees
No one assigned
Labels
graph-changing This change breaks the current graph structure, i.e. it changes semantic of properties and edges
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@maximiliankaul @KuechA @oxisto