'Force Java compilation' breaks diagnostics in dirty files

[snjeza]

Steps to reproduce:

• open a Java project
• ensure that there is no diagnostic message
• edit a Java file so you get a diagnostic message
• do not save the file
• call 'Force Java compilation'>Full

Diagnostic message disappears.

A solution would be to:

• require a user to save all dirty files when calling 'Force Java compilation'
  or
• revalidate opened compilation units after building the workspace

[tsmaeder]

Does this scenario work in Eclipse?

require a user to save all dirty files when calling 'Force Java compilation'
or

Sorry, but no can do: we don't control the clients.

[snjeza]

Does this scenario work in Eclipse?

It does.

[fbricon]

actually it doesn't work in eclipse either.
Let's say you have Foo and Bar, with Bar extends Foo.
If both files are opened in Eclipse, rename Foo to NewFoo, Bar will extends NewFoo, won't appear as modified.
Now if Bar is dirty, rename Foo to NewFoo, then you'll see an error in Bar saying Foo cannot be resolved to a type
What works is if Bar is dirty, rename Foo to NewFoo directly from within Bar, then no errors will show up after. Bar will be saved

[fbricon]

revalidate opened compilation units after building the workspace

could work, but I'm afraid that might cause a performance issue in the case of incremental builds when several hundreds of files are opened.
Might be worth pursuing though, at least to fix @yaohaizh's problem with renaming support in #840

[yaohaizh]

@fbricon @snjeza In order to fix the diagnostic issue, I make some try in this commits: 3dc6a0c

It did fix the issue for rename type, but still not fixed the issue for renaming packages. So I would think we should save the files when renaming, which is the behavior of the eclipse.

Similar case for "Force Java Compiling"

[fbricon]

In order to fix the diagnostic issue, I make some try in this commits: 3dc6a0c

can we get unit tests for that?

[fbricon]

This seems fixed by @yaohaizh's PR #864. However, I'd like to see unit tests to ensure future non-regression

[snjeza]

This seems fixed by @yaohaizh's PR #864. However, I'd like to see unit tests to ensure future non-regression

@yaohaizh has already created DocumentLifeCycleHandlerTest.testBasicBufferLifeCycleWithoutSave - See https://github.com/eclipse/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/DocumentLifeCycleHandlerTest.java#L250