Don't refresh files when they are not notified as changed.

The refresh was causing "Cycle loading state error", so it is possible
that this CL will fix this long standing issue.

Bug: https://github.com/dart-lang/sdk/issues/43073
Change-Id: Id1eeacd01cf10e918b002d227c8942e38bed543c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/159140
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This commit is contained in:
Konstantin Shcheglov 2020-08-18 21:15:00 +00:00 committed by commit-bot@chromium.org
parent 518f96a70a
commit 1c5a17b212
3 changed files with 181 additions and 31 deletions

View file

@ -587,7 +587,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
/// The [path] must be absolute and normalized.
FileResult getFileSync(String path) {
_throwIfNotAbsolutePath(path);
FileState file = _fileTracker.verifyApiSignature(path);
FileState file = _fileTracker.getFile(path);
return FileResultImpl(
_currentSession, path, file.uri, file.lineInfo, file.isPart);
}
@ -819,7 +819,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
Future<SourceKind> getSourceKind(String path) async {
_throwIfNotAbsolutePath(path);
if (AnalysisEngine.isDartFileName(path)) {
FileState file = _fileTracker.verifyApiSignature(path);
FileState file = _fileTracker.getFile(path);
return file.isPart ? SourceKind.PART : SourceKind.LIBRARY;
}
return null;
@ -907,7 +907,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
/// resolved unit).
ParsedUnitResult parseFileSync(String path) {
_throwIfNotAbsolutePath(path);
FileState file = _fileTracker.verifyApiSignature(path);
FileState file = _fileTracker.getFile(path);
RecordingErrorListener listener = RecordingErrorListener();
CompilationUnit unit = file.parse(listener);
return ParsedUnitResultImpl(currentSession, file.path, file.uri,

View file

@ -139,6 +139,18 @@ class FileTracker {
_pendingFiles.remove(path);
}
/// Return the [FileState] suitable only when the file is used by itself.
/// If the file is in the set of changed files, it is refreshed first.
/// If the file is not in the set of changed files, it is not refreshed.
FileState getFile(String path) {
// Read any files that we were told were changed.
// But don't read the requested file explicitly.
// Read it only if it is in this set of changed files.
while (verifyChangedFilesIfNeeded()) {}
return _fsState.getFileForPath(path);
}
/// Returns a boolean indicating whether the given [path] points to a file
/// that requires analysis.
bool isFilePending(String path) {

View file

@ -1275,6 +1275,43 @@ bbb() {}
expect(files, isNot(contains(c)));
}
test_getFileSync_changedFile() async {
var a = convertPath('/test/lib/a.dart');
var b = convertPath('/test/lib/b.dart');
newFile(a, content: '');
newFile(b, content: r'''
import 'a.dart';
void f(A a) {}
''');
// Ensure that [a.dart] library cycle is loaded.
// So, `a.dart` is in the library context.
await driver.getResult(a);
// Update the file, changing its API signature.
// Note that we don't call `changeFile`.
newFile(a, content: 'class A {}\n');
// Get the file.
// We have not called `changeFile(a)`, so we should not read the file.
// Moreover, doing this will create a new library cycle [a.dart].
// Library cycles are compared by their identity, so we would try to
// reload linked summary for [a.dart], and crash.
expect(driver.getFileSync(a).lineInfo.lineCount, 1);
// We have not read `a.dart`, so `A` is still not declared.
expect((await driver.getResult(b)).errors, isNotEmpty);
// Notify the driver that the file was changed.
driver.changeFile(a);
// So, `class A {}` is declared now.
expect(driver.getFileSync(a).lineInfo.lineCount, 2);
expect((await driver.getResult(b)).errors, isEmpty);
}
test_getFileSync_library() async {
var path = convertPath('/test/lib/a.dart');
newFile(path);
@ -1825,6 +1862,43 @@ var A2 = B1;
expect(result1.unit, isNotNull);
}
test_getSourceKind_changedFile() async {
var a = convertPath('/test/lib/a.dart');
var b = convertPath('/test/lib/b.dart');
newFile(a, content: 'part of lib;');
newFile(b, content: r'''
import 'a.dart';
void f(A a) {}
''');
// Ensure that [a.dart] library cycle is loaded.
// So, `a.dart` is in the library context.
await driver.getResult(a);
// Update the file, changing its API signature.
// Note that we don't call `changeFile`.
newFile(a, content: 'class A {}');
// Get the kind of the file.
// We have not called `changeFile(a)`, so we should not read the file.
// Moreover, doing this will create a new library cycle [a.dart].
// Library cycles are compared by their identity, so we would try to
// reload linked summary for [a.dart], and crash.
expect(await driver.getSourceKind(a), SourceKind.PART);
// We have not read `a.dart`, so `A` is still not declared.
expect((await driver.getResult(b)).errors, isNotEmpty);
// Notify the driver that the file was changed.
driver.changeFile(a);
// So, `class A {}` is declared now.
expect(await driver.getSourceKind(a), SourceKind.LIBRARY);
expect((await driver.getResult(b)).errors, isEmpty);
}
test_getSourceKind_changeFile() async {
var path = convertPath('/test/lib/test.dart');
expect(await driver.getSourceKind(path), SourceKind.LIBRARY);
@ -2164,6 +2238,55 @@ import 'b.dart';
expect(error.errorCode, CompileTimeErrorCode.MISSING_DART_LIBRARY);
}
test_parseFile_changedFile() async {
var a = convertPath('/test/lib/a.dart');
var b = convertPath('/test/lib/b.dart');
newFile(a, content: '');
newFile(b, content: r'''
import 'a.dart';
void f(A a) {}
''');
// Ensure that [a.dart] library cycle is loaded.
// So, `a.dart` is in the library context.
await driver.getResult(a);
// Update the file, changing its API signature.
// Note that we don't call `changeFile`.
newFile(a, content: 'class A {}');
// Parse the file.
// We have not called `changeFile(a)`, so we should not read the file.
// Moreover, doing this will create a new library cycle [a.dart].
// Library cycles are compared by their identity, so we would try to
// reload linked summary for [a.dart], and crash.
{
var parseResult = await driver.parseFile(a);
expect(parseResult.unit.declarations, isEmpty);
}
// We have not read `a.dart`, so `A` is still not declared.
{
var bResult = await driver.getResult(b);
expect(bResult.errors, isNotEmpty);
}
// Notify the driver that the file was changed.
driver.changeFile(a);
// So, `class A {}` is declared now.
{
var parseResult = driver.parseFileSync(a);
expect(parseResult.unit.declarations, hasLength(1));
}
{
var bResult = await driver.getResult(b);
expect(bResult.errors, isEmpty);
}
}
test_parseFile_notAbsolutePath() async {
expect(() async {
await driver.parseFile('not_absolute.dart');
@ -2179,21 +2302,53 @@ import 'b.dart';
expect(driver.knownFiles, contains(p));
}
test_parseFile_shouldRefresh() async {
var p = convertPath('/test/bin/a.dart');
test_parseFileSync_changedFile() async {
var a = convertPath('/test/lib/a.dart');
var b = convertPath('/test/lib/b.dart');
newFile(p, content: 'class A {}');
driver.addFile(p);
newFile(a, content: '');
newFile(b, content: r'''
import 'a.dart';
// Get the result, so force the file reading.
await driver.getResult(p);
void f(A a) {}
''');
// Update the file.
newFile(p, content: 'class A2 {}');
// Ensure that [a.dart] library cycle is loaded.
// So, `a.dart` is in the library context.
await driver.getResult(a);
ParsedUnitResult parseResult = await driver.parseFile(p);
var clazz = parseResult.unit.declarations[0] as ClassDeclaration;
expect(clazz.name.name, 'A2');
// Update the file, changing its API signature.
// Note that we don't call `changeFile`.
newFile(a, content: 'class A {}');
// Parse the file.
// We have not called `changeFile(a)`, so we should not read the file.
// Moreover, doing this will create a new library cycle [a.dart].
// Library cycles are compared by their identity, so we would try to
// reload linked summary for [a.dart], and crash.
{
var parseResult = driver.parseFileSync(a);
expect(parseResult.unit.declarations, isEmpty);
}
// We have not read `a.dart`, so `A` is still not declared.
{
var bResult = await driver.getResult(b);
expect(bResult.errors, isNotEmpty);
}
// Notify the driver that the file was changed.
driver.changeFile(a);
// So, `class A {}` is declared now.
{
var parseResult = driver.parseFileSync(a);
expect(parseResult.unit.declarations, hasLength(1));
}
{
var bResult = await driver.getResult(b);
expect(bResult.errors, isEmpty);
}
}
test_parseFileSync_languageVersion() async {
@ -2236,23 +2391,6 @@ class A {}
expect(driver.knownFiles, contains(p));
}
test_parseFileSync_shouldRefresh() async {
var p = convertPath('/test/bin/a.dart');
newFile(p, content: 'class A {}');
driver.addFile(p);
// Get the result, so force the file reading.
await driver.getResult(p);
// Update the file.
newFile(p, content: 'class A2 {}');
ParsedUnitResult parseResult = driver.parseFileSync(p);
var clazz = parseResult.unit.declarations[0] as ClassDeclaration;
expect(clazz.name.name, 'A2');
}
test_part_getErrors_afterLibrary() async {
var a = convertPath('/test/lib/a.dart');
var b = convertPath('/test/lib/b.dart');