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

Introduce Python plugin #549

Open
wants to merge 45 commits into
base: master
Choose a base branch
from
Open

Conversation

rmfcnb
Copy link

@rmfcnb rmfcnb commented Nov 29, 2021

No description provided.

rmfcnb and others added 30 commits January 14, 2021 23:54
Set AstNodeInfo entity hash in service.
Fix python dependecnies and narrowing error.
Fix declaration inside except block
Handled starred variable in with statement
It seems like the original source has gone. The project https://github.com/threatstack/libmagic/ was deleted or made private.
Introduce process_file_content.
rmfcnb and others added 10 commits October 31, 2021 18:22
Remove unnecessary inner class usage persisting.
# Conflicts:
#	.gitlab/build-codecompass.sh
#	.gitlab/cc-env.sh
#	webgui/scripts/codecompass/view/codeBites.js
#	webgui/scripts/codecompass/view/component/Text.js
…Compass into rmfcnb-pythonplugin-symlink

# Conflicts:
#	plugins/python/parser/src/scripts/cc_python_parser/parser.py
Remove unnecessary inner class usage persisting.
Add missing imported declaration to ImportedDeclaration
@intjftw intjftw added Kind: Enhancement 🌟 Kind: Important 🥇 Plugin: Python Issues related to the parsing and presentation of Python projects. labels Nov 29, 2021
@mcserep mcserep added this to the Release Gershwin milestone Jan 23, 2022
@mcserep mcserep force-pushed the master branch 2 times, most recently from bcca4c9 to 52327cd Compare January 25, 2022 09:18
plugins/python/service/include/service/pythonservice.h Outdated Show resolved Hide resolved
plugins/python/parser/src/pythonparser.cpp Outdated Show resolved Hide resolved
plugins/python/parser/src/pythonparser.cpp Outdated Show resolved Hide resolved
plugins/python/parser/src/pythonparser.cpp Outdated Show resolved Hide resolved
plugins/python/parser/src/pythonparser.cpp Outdated Show resolved Hide resolved
plugins/python/parser/src/pythonparser.cpp Outdated Show resolved Hide resolved
plugins/python/parser/src/pythonparser.cpp Outdated Show resolved Hide resolved
plugins/python/parser/src/pythonparser.cpp Outdated Show resolved Hide resolved
plugins/python/parser/src/pythonparser.cpp Outdated Show resolved Hide resolved
plugins/python/parser/src/pythonparser.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@bruntib bruntib left a comment

Choose a reason for hiding this comment

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

First, I'd like to tell that this Python parser is a great work, I appreciate it. I think we should continue the review process and this will be a good language plugin.


class HashableList(Generic[T], List[T]):
def __hash__(self):
return hash(e for e in self)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return hash(e for e in self)
return hash(tuple(e for e in self))

In the previous version a generator object was hashed, so in practice every HashableList object had the same hash value. We should create a tuple instead which is hashable and the hash value is really generated based on its members.

I think, however, that we should eliminate this HashableList completely. In Python only hashable objects can be used for dictionary keys. And this HashableList is used in this context. But it's not an elegant way of using lists as keys, because changing list elements ruins the dictionary. The only difference between tuple and list in Python is that tuples are not mutable thus can be used as keys. I think we should introduce this change:

# This is the current usage.
my_list = HashableList()
my_dict = {my_list: 42}

# We should do this.
my_list = []
my_dict = {tuple(my_list): 42}

Conversion to tuple would happen in this __hash__() function anyways, so it is not a performance loss. I haven't reviewed the part here this HashableList is used, so I'm not sure if using a tuple as key is really necessary.

Copy link
Collaborator

@mcserep mcserep left a comment

Choose a reason for hiding this comment

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

I have tested functionally the Python plugin with the following single-file Python script:
https://gist.github.com/mcserep/9df2edbb6767b9190e438f9ed7f3a944

Remarks for the parser:

  • Parsing went smoothly, although took more time than expected. (requests and datetime modules were imported.)
  • During parsing a lot of debug messages were shown, which should be hidden by default.

Remarks for the service:

  • Multiline documentation comments are not handled properly and treated as Python code (Invalid PythonAstNode ID message shown when clicking inside).
  • Clicking on the audit_submission function works properly, but clicking on the get_user_name function results in segfault.
  • Clicking on imported symbols (e.g. requests or requests.get) also displays an Invalid PythonAstNode ID.
  • CodeBites diagram is not shown properly.

rmfcnb added 3 commits April 23, 2022 20:47
Introduce skip file
Small refactors
Introduce Interpret BuildAction type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kind: Enhancement 🌟 Plugin: Python Issues related to the parsing and presentation of Python projects.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants