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

fix: fix modeling bug #5912

Merged
merged 4 commits into from
Aug 10, 2024
Merged

fix: fix modeling bug #5912

merged 4 commits into from
Aug 10, 2024

Conversation

monperrus
Copy link
Collaborator

expected behavior: buf is a field access

actual behavior: buf is a typeaccess

cause: isResolvedField assumed that everything is source-available and returned false if a library class (binary only, ByteArrayOutputStream here)

Copy link
Collaborator

@SirYwell SirYwell left a comment

Choose a reason for hiding this comment

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

From my understanding, this was only a problem in no classpath mode?

@monperrus
Copy link
Collaborator Author

monperrus commented Aug 8, 2024

hi @SirYwell thanks for your review.

no, the serious bug is in fullclasspath, see https://github.com/INRIA/spoon/pull/5912/files#diff-be3ade2426b2d0edf889e62dc3e857c91c4c5935d502b792d7a751800fd327eaR181

so we want a fully correct model in fullclasspath, and OKish behavior in noclasspath (this is the change in the other test)

@SirYwell
Copy link
Collaborator

SirYwell commented Aug 8, 2024

Thanks for clarifying.

Now, the model represents an implicit this.<field> access. In the mentioned code however, I wonder if it would make more sense to model it as super.<field> access, as the field is actually declared in a super class. I'm not sure how relevant that is for analysis, but I could imagine that it is relevant for transformations if they shadow a field (although that might break in even more places, e.g. printing).

I guess that's out of scope for this PR, but it's something that came to my mind seeing the code.

@monperrus
Copy link
Collaborator Author

great, thanks @SirYwell . let's merge then to have it in the next beta

@SirYwell SirYwell merged commit 44e3b21 into master Aug 10, 2024
13 checks passed
@SirYwell SirYwell deleted the bug-guava branch August 10, 2024 19:03
@monperrus
Copy link
Collaborator Author

And now Guava can be spooned again: https://ci.inria.fr/sos/job/Guava/

Thanks @SirYwell

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