-
Notifications
You must be signed in to change notification settings - Fork 8
Java transitive search constructor support #169
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
Java transitive search constructor support #169
Conversation
Signed-off-by: Theodor Mihalache <tmihalac@tmihalac-thinkpadp1gen7.rmtusfl.csb>
Signed-off-by: Theodor Mihalache <tmihalac@tmihalac-thinkpadp1gen7.rmtusfl.csb>
Signed-off-by: Theodor Mihalache <tmihalac@tmihalac-thinkpadp1gen7.rmtusfl.csb>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmihalac Great job.
just 2 minor comments.
Please handle, and we'll merge it.
Thanks.
| "native","strictfp","default","transient","volatile","sealed" | ||
| # 'non-sealed' is two tokens; irrelevant for methods here | ||
| } | ||
| JAVA_ANNOTATION_SYMBOL = '@' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmihalac Why not reusing it by importing it from
| JAVA_ANNOTATION_SYMBOL = '@' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right
|
|
||
| elif state == LINE: | ||
| nl = s.find("\n", i) | ||
| nl = find("\n", i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmihalac This is not clear at all and very confusing , you're using here ( and in more places in the code) a local variable of the top level function is_java_type which containing an object' method ( in this case it's find method of a string), i would recommend granting to this variable a better name like find_in_source and refactor it accordingly ( to understand it in the code review , this alone took me few minutes), as the name find is too general and can be ambiguous easily.
|
|
||
| def is_ws(ch: str) -> bool: | ||
| return ch <= " " and ch in " \t\r\n\f\v" | ||
| find = s.find |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| find = s.find | |
| find_in_source = s.find |
Signed-off-by: Theodor Mihalache <tmihalac@tmihalac-thinkpadp1gen7.rmtusfl.csb>
Signed-off-by: Theodor Mihalache <tmihalac@tmihalac-thinkpadp1gen7.rmtusfl.csb>
|
/retest vulnerability-analysis-on-pr |
zvigrinberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Approved.
* Added constructor support to java transitive search Signed-off-by: Theodor Mihalache <tmihalac@tmihalac-thinkpadp1gen7.rmtusfl.csb>
No description provided.