Context invalidation on option changes (Implements #24574).

Context invalidation on option changes (Implements #24574).

R=brianwilkerson@google.com, scheglov@google.com

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

[pquitslund, 2015-10-14 23:47:50]

pquitslund@google.com changed reviewers:
+ brianwilkerson@google.com, scheglov@google.com

[pquitslund, 2015-10-14 23:47:51]

It's been a long day so it's likely I left something out.  Nonetheless, the
basic bits are there and ready for some feedback.

Thanks in advance!

[Brian Wilkerson, 2015-10-15 00:20:36]

LGTM

https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/src/context_manager.dart (right):

https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/context_manager.dart:317: static const String
ANALYSIS_OPTIONS_FILE = '.analysis_options';
Do we have other definitions of this constant (or other references to this file
name)? If so, it would be nice to unify them.

https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/context_manager.dart:523: if (strongMode is bool) {
If the key isn't there I think we want to treat it as "false". Hence, we
probably want to do something like "strongMode ??= false;" before the test.

[scheglov, 2015-10-15 00:25:45]

LGTM

[pquitslund, 2015-10-15 03:39:06]

Thanks!

https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/src/context_manager.dart (right):

https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/context_manager.dart:317: static const String
ANALYSIS_OPTIONS_FILE = '.analysis_options';
On 2015/10/15 00:20:36, Brian Wilkerson wrote:
> Do we have other definitions of this constant (or other references to this
file
> name)? If so, it would be nice to unify them.

Agreed.  The only other ones are in tests and in analyzer (and probably not
where they should live long term).  Do you have a convention for where constants
such as these should live?

https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/context_manager.dart:523: if (strongMode is bool) {
On 2015/10/15 00:20:36, Brian Wilkerson wrote:
> If the key isn't there I think we want to treat it as "false". Hence, we
> probably want to do something like "strongMode ??= false;" before the test.

Ah.  Good catch!   I took this approach:

    bool strongMode = analyzer['strong-mode'] ?? false;

Thanks!

[pquitslund, 2015-10-15 03:42:56]

Committed patchset #3 (id:40001) manually as
cae05d6bd43d8f2cf616b6d93317c28a99778190 (presubmit successful).

[Brian Wilkerson, 2015-10-15 13:15:31]

https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/src/context_manager.dart (right):

https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/context_manager.dart:317: static const String
ANALYSIS_OPTIONS_FILE = '.analysis_options';
> Do you have a convention for where constants such as these should live?

Not really, no.

[pquitslund, 2015-10-19 16:38:00]

On 2015/10/15 13:15:31, Brian Wilkerson wrote:
>
https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
> File pkg/analysis_server/lib/src/context_manager.dart (right):
> 
>
https://codereview.chromium.org/1399893004/diff/20001/pkg/analysis_server/lib...
> pkg/analysis_server/lib/src/context_manager.dart:317: static const String
> ANALYSIS_OPTIONS_FILE = '.analysis_options';
> > Do you have a convention for where constants such as these should live?
> 
> Not really, no.

https://codereview.chromium.org/1412213003/