Record dependencies and invalidate results.

Record dependencies and invalidate results.

R=brianwilkerson@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=45480

[Brian Wilkerson, 2015-05-03 15:35:33]

LGTM

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/lib/src/contex...
File pkg/analyzer/lib/src/context/cache.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/lib/src/contex...
pkg/analyzer/lib/src/context/cache.dart:846: void
_addDependentResult(TargetedResult result) {
I don't understand why we would make these private methods. Instances of
ResultData cannot escape to the public API, so there's no reason I can see for
making the code harder to read. Is there something I'm missing?

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/abstract_context_test.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/abstract_context_test.dart:38: * @param elements
the elements
Perhaps convert to a Dart-style comment while we're at it?

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/cache_test.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/cache_test.dart:31: AnalysisCache
_createCache({AnalysisContext context,
Is there a reason for making this private?

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/cache_test.dart:294:
expect(entry.getState(result2), CacheState.INVALID);
I'd need to look at the code or write some test code to confirm, but I believe
that at this point if we attempted to compute result2, we'd discover that
result1 is in ERROR and propagate that error to result2. I'm not sure that's the
right behavior, but I do think we should be consistent. Let's investigate.

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/context_test.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/context_test.dart:1941: void
fail_getLaunchableClientLibrarySources_importsHtml_explicitly() {
Any idea why these tests started failing?

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/mock_sdk.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/mock_sdk.dart:15: //import
'package:analyzer/src/generated/engine.dart';
Did you intend to remove this line?

[scheglov, 2015-05-03 20:26:28]

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/lib/src/contex...
File pkg/analyzer/lib/src/context/cache.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/lib/src/contex...
pkg/analyzer/lib/src/context/cache.dart:846: void
_addDependentResult(TargetedResult result) {
On 2015/05/03 15:35:32, Brian Wilkerson wrote:
> I don't understand why we would make these private methods. Instances of
> ResultData cannot escape to the public API, so there's no reason I can see for
> making the code harder to read. Is there something I'm missing?

No, you're right.
I will make these new methods and fields public.

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/abstract_context_test.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/abstract_context_test.dart:38: * @param elements
the elements
On 2015/05/03 15:35:32, Brian Wilkerson wrote:
> Perhaps convert to a Dart-style comment while we're at it?

Done.

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/cache_test.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/cache_test.dart:31: AnalysisCache
_createCache({AnalysisContext context,
On 2015/05/03 15:35:33, Brian Wilkerson wrote:
> Is there a reason for making this private?

No, nothing new.
I will make it public.

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/cache_test.dart:294:
expect(entry.getState(result2), CacheState.INVALID);
On 2015/05/03 15:35:33, Brian Wilkerson wrote:
> I'd need to look at the code or write some test code to confirm, but I believe
> that at this point if we attempted to compute result2, we'd discover that
> result1 is in ERROR and propagate that error to result2. I'm not sure that's
the
> right behavior, but I do think we should be consistent. Let's investigate.

Yes, I was pondering this too.
Done.

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/context_test.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/context_test.dart:1941: void
fail_getLaunchableClientLibrarySources_importsHtml_explicitly() {
On 2015/05/03 15:35:33, Brian Wilkerson wrote:
> Any idea why these tests started failing?

It was temporary.
After rebasing on your changes it passes again.

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/mock_sdk.dart (right):

https://codereview.chromium.org/1121963002/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/mock_sdk.dart:15: //import
'package:analyzer/src/generated/engine.dart';
On 2015/05/03 15:35:33, Brian Wilkerson wrote:
> Did you intend to remove this line?

Acknowledged.

[scheglov, 2015-05-03 20:27:06]

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