-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add static method #42
Conversation
Professor, When I was writing the paper for Specimin, I realized that I forgot to handle static method calls in the input program, which is quite embarrassing. This pull request addresses that by adding the necessary code for unsolved static method calls. Additionally, I noticed the existence of the In this updated version of Specimin, wildcard imports are no longer allowed. The reason is that we need to distinguish between class and method names in unsolved method calls, such as I think the logic of this solution might not apply if there is a method or class instance with the same name as a class among the import statements. But I think the odd is low, since usually people don't name methods or class instances with a capitalized name like they do for classes. |
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.
Nice catch! Please also add a test for a more complex static call (e.g., one where the programmer uses the fully-qualified name of the class).
src/test/resources/unsolvedstaticmethod/expected/com/example/Class.java
Outdated
Show resolved
Hide resolved
Professor, I've made some changes to the code. My initial implementation was incorrect. The problem was somewhat concealed because I named my test class I've also introduced a more complex method call in the test file. Please take a look. Thank you. |
src/test/java/org/checkerframework/specimin/UnsolvedStaticMethod.java
Outdated
Show resolved
Hide resolved
src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java
Outdated
Show resolved
Hide resolved
Professor, I have updated the codes and the test file. |
I don't know why the CI is not running. I think this is a bug of GitHub. But the solutions discussed in that bug report don't seem to be related to our case. |
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.
I'm still concerned about the string manipulation.
Also, I'm not sure that this code will handle static imports correctly. Can you also add a test that statically imports a method and make sure that the code handles it properly?
src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java
Outdated
Show resolved
Hide resolved
src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java
Outdated
Show resolved
Hide resolved
Professor,
I have renamed the related methods and added clarification in the Javadoc to make sure that the expected input will be passed to this string manipulation call.
Unfortunately, I wasn't aware of this Java feature until today. Updating Specimin for static imports might involve extensive modifications, such as changing how it reads import statements and updates |
They are rare, so that's fine. Please open an issue for this so we don't forget. |
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.
When the requested changes are implemented, you can merge without requiring another review from me.
No description provided.