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: prevent ignorance of initial changes #1720

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

dos65
Copy link
Collaborator

@dos65 dos65 commented Apr 12, 2022

No description provided.

@@ -84,7 +84,10 @@ private final class BloopNameHashing(
initialChangedSources,
allSources.map(v => v: VirtualFileRef),
previous
).collect { case f: VirtualFile => f }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

initialChagedSources are received as VirtualFileRef and mapInvalidationToSources doesn't lift them into VirtualFile.
Using collect here is incorrect as it just drops correctly invalidated sources.

Copy link
Contributor

Choose a reason for hiding this comment

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

och! That looks like the issue i've been having! Awesom!

Comment on lines +87 to +90
).map {
case f: VirtualFile => f
case ref: VirtualFileRef => PlainVirtualFileConverter.converter.toVirtualFile(ref)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tgodzik Is PlainVirtualFileConverter.converter usage fine here?
I've seen that in some methods converter is passed as parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so, we don't use any other converters currently, so this should be fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

what is the actual type returned? VirtualFileRef is only an interface, no?

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +87 to +90
).map {
case f: VirtualFile => f
case ref: VirtualFileRef => PlainVirtualFileConverter.converter.toVirtualFile(ref)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think so, we don't use any other converters currently, so this should be fine.

@@ -84,7 +84,10 @@ private final class BloopNameHashing(
initialChangedSources,
allSources.map(v => v: VirtualFileRef),
previous
).collect { case f: VirtualFile => f }
Copy link
Contributor

Choose a reason for hiding this comment

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

och! That looks like the issue i've been having! Awesom!

@tgodzik tgodzik merged commit 805134a into scalacenter:main Apr 12, 2022
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