-
Notifications
You must be signed in to change notification settings - Fork 629
Fixing #3159 - Method call graph ignores constructors and non-parametric void methods #3160
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
Conversation
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.
Thank you for your contribution!
Can you please add one more test, where one of the functions called multiple times in a way that it could have affect on the order?
Can you please add an entry to the CHANGELOG.md
file's unreleased part?
spotbugs-tests/src/test/java/edu/umd/cs/findbugs/classfile/MethodsCallOrderTest.java
Outdated
Show resolved
Hide resolved
spotbugs/src/main/java/edu/umd/cs/findbugs/classfile/engine/SelfMethodCalls.java
Outdated
Show resolved
Hide resolved
Hi @JuditKnoll , just to confirm my understanding, I can think of 2 different kinds that impact the order. First case is when Another case is Thank you |
I was thinking about the second scenario. |
Hi @JuditKnoll, please see the new commits. Thanks. |
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.
Can you please add an entry to the CHANGELOG.md
file's unreleased part?
Ops, forgot. Thank you @JuditKnoll. |
…d methods. All these methods are missing to be properly sorted by TopologicalSort (spotbugs#3159)
4cae481
to
085de4f
Compare
There was a conflict in CHANGELOG.md. Rebased. |
Looks like the problem is there from the very beginnings where the logic ignored all non-parametric methods as "not interesting".
Omitting those methods from the MultiMap doesn't give TopologySort any information about the graph orientation and these methods are randomly misplaced when calling
ClassContext#getMethodsInCallOrder
.Part of the PR is a test that has a simple call stack from the default
static{}
constructor<clinit>()V
through several static methods to a non-parametric class constructor, one more constructor and then instantiated class methods which creates a very simple DAG that any topological sorting algorithm should have no problem analyzing.After fixing the
interestingSignature
method fromreturn !"()V".equals(signature);
toreturn true;
, it works! :)FYI: required for proper FindSecurityBugs taint analysis of derived methods. Without proper order they are skipped by the analysis engine as unknown.