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

Add static method #42

Merged
merged 10 commits into from
Nov 15, 2023
Merged

Conversation

LoiNguyenCS
Copy link
Collaborator

No description provided.

@LoiNguyenCS
Copy link
Collaborator Author

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 tempDirJDK folder, which I had created a while ago for bug testing purposes. I believe it was accidentally added at some point in the past, so this PR includes the removal of that folder.

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 A.c(). The only reliable way to determine whether A refers to a class or a method is to look at the import statements.

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.

@LoiNguyenCS LoiNguyenCS requested a review from kelloggm November 13, 2023 21:00
Copy link
Collaborator

@kelloggm kelloggm left a 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).

@LoiNguyenCS
Copy link
Collaborator Author

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 Class, coincidentally matching some built-in Java classes. Thus, the program ran without triggering any error.

I've also introduced a more complex method call in the test file.

Please take a look. Thank you.

@LoiNguyenCS LoiNguyenCS requested a review from kelloggm November 14, 2023 02:57
@LoiNguyenCS
Copy link
Collaborator Author

Professor,

I have updated the codes and the test file.

@LoiNguyenCS LoiNguyenCS requested a review from kelloggm November 14, 2023 17:42
@LoiNguyenCS
Copy link
Collaborator Author

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.

Copy link
Collaborator

@kelloggm kelloggm left a 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?

@LoiNguyenCS
Copy link
Collaborator Author

Professor,

I'm still concerned about the string manipulation.

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.

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?

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 classAndPackageMap. I suggest addressing this in a separate PR, ideally after resolving issue #41.
`

@kelloggm
Copy link
Collaborator

Updating Specimin for static imports might involve extensive modifications, such as changing how it reads import statements and updates classAndPackageMap. I suggest addressing this in a separate PR, ideally after resolving issue #41.

They are rare, so that's fine. Please open an issue for this so we don't forget.

Copy link
Collaborator

@kelloggm kelloggm left a 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.

@LoiNguyenCS LoiNguyenCS merged commit 40572c2 into njit-jerse:main Nov 15, 2023
1 check passed
@LoiNguyenCS LoiNguyenCS deleted the add-static-method branch November 15, 2023 02:03
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.

2 participants