Skip to content
New issue

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

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

Already on GitHub? # to your account

Cleanup of SymbolResolver #1777

Merged
merged 9 commits into from
Oct 4, 2024
Merged

Cleanup of SymbolResolver #1777

merged 9 commits into from
Oct 4, 2024

Conversation

oxisto
Copy link
Member

@oxisto oxisto commented Oct 3, 2024

Unfortunatly, this cannot really be split into smaller pieces. It basically does the following:

  • Move all inference related functions out of the symbol resolver and into a separate helper file
  • Remove unnecessary calls to resolve member expressions in handleReference

I can try to add the needed language trait in a separate PR (see #1778). This also needs smaller adjustments in the python and typescript frontend. Mostly tests that previously asserted the "wrong" thing, are now asserting the correct resolution.

Fixes #1769

oxisto added a commit that referenced this pull request Oct 3, 2024
This is needed in preparation for #1777 to better handle access to fields/members of a class when an implicit receiver is used.
Copy link

codecov bot commented Oct 3, 2024

oxisto added a commit that referenced this pull request Oct 3, 2024
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 added 3 commits October 3, 2024 22:11
Unfortunatly, this cannot really be split into smaller pieces. It basically does the following:
- Move all inference related functions out of the symbol resolve
- Try to restrict / remove calls to the legacy `resolveReference` function

Fixes #1769
Fixes #1771
@oxisto oxisto force-pushed the cleanup-symbol-resolver branch from 7339e6a to ada1c2b Compare October 3, 2024 20:11
Copy link
Contributor

@KuechA KuechA left a comment

Choose a reason for hiding this comment

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

I think that it would be great if we could document some of the assumptions and the expected results. Doesn't necessarily have to be in the kdoc of the methods but a md for the docs page would be a good option too

@oxisto oxisto merged commit c27b13d into main Oct 4, 2024
4 checks passed
@oxisto oxisto deleted the cleanup-symbol-resolver branch October 4, 2024 13:07
maximiliankaul pushed a commit that referenced this pull request Oct 8, 2024
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.
maximiliankaul pushed a commit that referenced this pull request Oct 8, 2024
KuechA added a commit that referenced this pull request Oct 18, 2024
* new node for RaiseStatement + NodeBuilder code

* don't add python stuff in this branch

* started work on EOG and DFG

* doc

* raise: fluent and first test

* fluent: patch @oxisto

* Start work on DFG test for raise

* rename raise -> throw

* more renaming raise -> throw

* copy & paste handleThrowOperator

* Update cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/edges/flows/DataflowTest.kt

Co-authored-by: KuechA <31155350+KuechA@users.noreply.github.com>

* Rename `findSymbols` into `lookupSymbolByName` (#1772)

* 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 <31155350+KuechA@users.noreply.github.com>

* Added documentation

---------

Co-authored-by: KuechA <31155350+KuechA@users.noreply.github.com>

* Update dependency rollup to v4.24.0 (#1774)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Added language trait `HasImplicitReceiver` (#1778)

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.

* Cleanup of `SymbolResolver` (#1777)

* Fixed crash in `getCodeOfSubregion` (#1776)

* Add new function `lookupUniqueTypeSymbolByName` (#1781)

* 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

* Make sure to move `typeObservers` from old to new node when replacing nodes (#1783)

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

* Added doc for typeobservers

* `implicit()` only triggers code/location update now if its not empty (#1784)

Otherwise, we override the code/location again.

* Added `:=` as simple operator in Python (#1785)

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.

* code review

* code review

* code review

* rename cause to parentException

* doc

* ThrowStatement: add toString

* fix tests

* Merge EOG, add tests

* Documentation

* More doc of EOG handling

---------

Co-authored-by: KuechA <31155350+KuechA@users.noreply.github.com>
Co-authored-by: Christian Banse <christian.banse@aisec.fraunhofer.de>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Alexander Kuechler <alexander.kuechler@aisec.fraunhofer.de>
KuechA added a commit that referenced this pull request Oct 18, 2024
* new node for RaiseStatement + NodeBuilder code

* don't add python stuff in this branch

* started work on EOG and DFG

* doc

* raise: fluent and first test

* fluent: patch @oxisto

* Start work on DFG test for raise

* rename raise -> throw

* more renaming raise -> throw

* python: raise

* copy & paste handleThrowOperator

* Update cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/edges/flows/DataflowTest.kt

Co-authored-by: KuechA <31155350+KuechA@users.noreply.github.com>

* Rename `findSymbols` into `lookupSymbolByName` (#1772)

* 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 <31155350+KuechA@users.noreply.github.com>

* Added documentation

---------

Co-authored-by: KuechA <31155350+KuechA@users.noreply.github.com>

* Update dependency rollup to v4.24.0 (#1774)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Added language trait `HasImplicitReceiver` (#1778)

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.

* Cleanup of `SymbolResolver` (#1777)

* Fixed crash in `getCodeOfSubregion` (#1776)

* Add new function `lookupUniqueTypeSymbolByName` (#1781)

* 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

* Make sure to move `typeObservers` from old to new node when replacing nodes (#1783)

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

* Added doc for typeobservers

* `implicit()` only triggers code/location update now if its not empty (#1784)

Otherwise, we override the code/location again.

* Added `:=` as simple operator in Python (#1785)

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.

* code review

* code review

* code review

* rename cause to parentException

* doc

* ThrowStatement: add toString

* fix tests

* Enable raising the exception in the with statement

* Test

* Test++

---------

Co-authored-by: KuechA <31155350+KuechA@users.noreply.github.com>
Co-authored-by: Christian Banse <christian.banse@aisec.fraunhofer.de>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Alexander Kuechler <alexander.kuechler@aisec.fraunhofer.de>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move inference helper files out of the symbol resolver
2 participants