-
Notifications
You must be signed in to change notification settings - Fork 29
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: multiple compilation units in a file lead to information loss #738
Conversation
@@ -545,6 +545,7 @@ lazy val docs = project | |||
.enablePlugins(DocusaurusPlugin) | |||
|
|||
lazy val javaOnlySettings = List[Def.Setting[_]]( | |||
javafmtOnCompile := false, |
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.
This was doing my head in – IntelliJ was formatting one way, running SBT session (~snapshots/run) another way, and IJ constantly producing a warning to load filesystem changes.
Formatting on compile is evil.
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.
Formatting on compile is evil.
Agreed.
// Upon first encounter with a file (before any other tasks are run) | ||
// we remove the semanticdb file for this source file to ensure | ||
// stale data doesn't cause problems |
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.
Now that we read previous state, we shouldn't expect the folder to be clean – without doing so I was getting very confusing results when running snapshots.
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.
LGTM 👍🏻 Very nice fix!
@@ -545,6 +545,7 @@ lazy val docs = project | |||
.enablePlugins(DocusaurusPlugin) | |||
|
|||
lazy val javaOnlySettings = List[Def.Setting[_]]( | |||
javafmtOnCompile := false, |
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.
Formatting on compile is evil.
Agreed.
Fixes GRAPH-753
We discovered that javac issues multiple events per file if there are multiple top level definitions that match what javac considers to be a compilation unit - interfaces or classes for example.
These multiple invocations produce slightly different results, and all writing to the same file, meaning the information gets lost.
To fix this, we defensively read the file if it already exists and carefully deduplicate symbols/occurrences/synthetics before writing back.
Test plan