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.

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

Make the filename property in pysrc2cpg relative again. #3544

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

ml86
Copy link
Contributor

@ml86 ml86 commented Aug 24, 2023

This the introduction of META_DATA.root, absolute path are not longer
required.

@DavidBakerEffendi can you please have a special look on the expectation i change in joern-cli/frontends/pysrc2cpg/src/test/scala/io/joern/pysrc2cpg/passes/TypeRecoveryPassTests.scala
To me it looks like it has been wrong in the first place because find_one should be external.

This the introduction of META_DATA.root, absolute path are not longer
required.
Copy link
Collaborator

@DavidBakerEffendi DavidBakerEffendi left a comment

Choose a reason for hiding this comment

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

LGTM, I'll just need to separately look at the external/internal thing in the type recovery.

@@ -459,7 +459,7 @@ class TypeRecoveryPassTests extends PySrc2CpgFixture(withOssDataflow = false) {

"correctly determine that, despite being unable to resolve the correct method full name, that it is an internal method" in {
val Some(selfFindFound) = cpg.typeDecl(".*InstallationsDAO.*").ast.isCall.name("find_one").headOption: @unchecked
selfFindFound.callee.isExternal.toSeq shouldBe Seq(true, false)
selfFindFound.callee.isExternal.toSeq shouldBe Seq(true, true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This contradicts the description of the test case, though I wonder what the cause of this is - perhaps import resolution is confused.

@DavidBakerEffendi
Copy link
Collaborator

I think go ahead with the merge and I'll make a separate PR to look into the type recovery part

@ml86 ml86 merged commit 86c3763 into master Aug 24, 2023
5 checks passed
DavidBakerEffendi added a commit that referenced this pull request Aug 28, 2023
As a follow up on this PR #3544 I investigated the claim of the unresolved method being intended as labelled isExternal=false.

I think it's likely a dangerous assumption to have in the first place with a path that has as many dummy access paths  as the test had. I'm happy to keep the expectation to be isExternal=true.
@max-leuthaeuser max-leuthaeuser deleted the markus/pysrc2cpgRelativeFileNames branch August 30, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants