Fix issue where errors aren't reported for a new overlay

Fix issue where errors aren't reported for a new overlay

A new overlay was not being marked as an explicitly added
source, even if it's within the directory of an analysis
context. As a result, errors were not being reported.

I believe this only happens if there's no underlying file.
Therefore it seems difficult to reproduce in an IDE, but
it comes up in a stress test.

BUG=
R=brianwilkerson@google.com

Committed: https://github.com/dart-lang/sdk/commit/5831cfb1bb2e5360cea6dd1850126515a8f3f7ce

[skybrian, 2016-03-02 03:55:18]

skybrian@google.com changed reviewers:
+ brianwilkerson@google.com

[skybrian, 2016-03-02 03:55:19]

I haven't written the tests yet, but I'd be interested in what you think of
this.

[skybrian, 2016-03-02 06:35:34]

https://codereview.chromium.org/1753083003/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/analysis_server.dart (right):

https://codereview.chromium.org/1753083003/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/analysis_server.dart:1267: explicit:
contextSource.contained)) {
Oops, this check should probably be something like:
  contextSource.context == context && contextSource.contained

[Brian Wilkerson, 2016-03-02 18:09:58]

I think it makes perfect sense for server to consider overlays for files that do
not exist to effectively mean that the file exists. If the file is in an
analysis root, and would normally be analyzed, then it should explicitly add it
to the appropriate context just like (I hope) we would do if we got a watcher
event indicating that a file had appeared.

We also need to make sure that if an overlay is removed and the file doesn't
exist that we remove the file from analysis just like we would do if we got a
watcher event indicating that a file had been deleted.

But I don't think we need (or ought) to make changes in analysis context to
support this. I'd rather see server explicitly call applyChanges to add or
remove sources from analysis.

[skybrian, 2016-03-03 02:26:06]

Could you take another look?

[Brian Wilkerson, 2016-03-03 14:53:05]

LGTM, although we need to remember to come back and remove the source if the
overlay is removed when the file does not exist.

[skybrian, 2016-03-03 18:43:05]

Description was changed from

==========
Fix issue where errors aren't reported for a new overlay

A new overlay was not being marked as an explicitly added
source, even if it's within the directory of an analysis
context. As a result, errors were not being reported.

I believe this only happens if there's no underlying file.
Therefore it seems difficult to reproduce in an IDE, but
it comes up in a stress test.

BUG=
==========

to

==========
Fix issue where errors aren't reported for a new overlay

A new overlay was not being marked as an explicitly added
source, even if it's within the directory of an analysis
context. As a result, errors were not being reported.

I believe this only happens if there's no underlying file.
Therefore it seems difficult to reproduce in an IDE, but
it comes up in a stress test.

BUG=
R=brianwilkerson@google.com

Committed:
https://github.com/dart-lang/sdk/commit/5831cfb1bb2e5360cea6dd1850126515a8f3f7ce
==========

[skybrian, 2016-03-03 18:43:07]

Committed patchset #2 (id:20001) manually as
5831cfb1bb2e5360cea6dd1850126515a8f3f7ce (presubmit successful).

[skybrian, 2016-03-03 18:45:40]

On 2016/03/03 14:53:05, Brian Wilkerson wrote:
> LGTM, although we need to remember to come back and remove the source if the
> overlay is removed when the file does not exist.

I think that's already handled (at line 1282). Also the stress test didn't find
anything.

[Brian Wilkerson, 2016-03-03 19:02:59]

Good. Then it would be useful to add an explicit test to avoid regressions.

[skybrian, 2016-03-03 19:43:04]

On 2016/03/03 19:02:59, Brian Wilkerson wrote:
> Good. Then it would be useful to add an explicit test to avoid regressions.

It looks like deletion is tested at the end of test_overlayOnly.