-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LibraryElement.featureSet
has non-nullable enabled for libraries in unmigrated packages when experiment flag is on
#43903
Comments
@leafpetersen @mit-mit |
Yes; we need the documentation to be accurate wrt. null safety. |
An update on this issue:
|
This test shows that we get Null Safety for one package, and legacy for another using @reflectiveTest
class NonNullableTest2 extends PubPackageResolutionTest {
@override
String get testPackageLanguageVersion => '2.11';
solo_test_class_hierarchy() async {
newFile('$workspaceRootPath/aaa/lib/a.dart', content: '');
newFile('$workspaceRootPath/test/lib/a.dart', content: '');
writeTestPackageConfig(
PackageConfigFileBuilder()
..add(name: 'aaa', rootPath: '$workspaceRootPath/aaa'),
);
writeTestPackageAnalysisOptionsFile(
AnalysisOptionsFileConfig(
experiments: [EnableString.non_nullable],
),
);
var result = await resolveFile('$workspaceRootPath/test/lib/a.dart');
print(
'[path: ${result.path}]'
'[isNullSafe: ${result.libraryElement.isNonNullableByDefault}]',
);
var result2 = await resolveFile('$workspaceRootPath/aaa/lib/a.dart');
print(
'[path: ${result2.path}]'
'[isNullSafe: ${result2.libraryElement.isNonNullableByDefault}]',
);
}
} Output:
|
@scheglov is there a reason to believe this behavior is different between AnalysisDriver and AnalysisContextCollection? |
No, it should work similar with A slightly modified test to show specifying Note that @reflectiveTest
class NonNullableTest2 extends PubPackageResolutionTest {
@override
String get testPackageLanguageVersion => '2.11';
solo_test_class_hierarchy() async {
newFile('/outer/aaa/lib/a.dart', content: '');
newFile('$workspaceRootPath/test/lib/a.dart', content: '');
writeTestPackageConfig(
PackageConfigFileBuilder()
..add(
name: 'aaa',
rootPath: '/outer/aaa',
languageVersion: '2.9',
),
);
writeTestPackageAnalysisOptionsFile(
AnalysisOptionsFileConfig(
experiments: [EnableString.non_nullable],
),
);
var result = await resolveFile('$workspaceRootPath/test/lib/a.dart');
print(
'[path: ${result.path}]'
'[isNullSafe: ${result.libraryElement.isNonNullableByDefault}]',
);
var result2 = await result.session.getResolvedUnit('/outer/aaa/lib/a.dart');
print(
'[path: ${result2.path}]'
'[isNullSafe: ${result2.libraryElement.isNonNullableByDefault}]',
);
}
} Output:
|
Well, that is actually the entire issue. We don't know that it is languageVersion 2.9 despite the test package being 2.11, I am depending on the analyzer to know this. |
i will build a test case for you. |
You don't need to "know" that a package is So far it seems to me that there are two solutions:
|
Note: we don't need to solve this for experiments (whether a |
@scheglov I am not sure I understand the meaning of your number 2. I don't think number 1 is needed. A single analysis context should be able to know this for libraries in different packages. After building package_config.json we already know the language versions for every package. I think it would be reasonable to map that to available feature sets for libraries in those packages. Maybe that is the same thing (or along the same lines) as your number 2? |
(1) is necessary if you enable an experiment, but your |
@scheglov My package_config.json for dartdoc seems to do so. e.g.:
|
Do you provide the |
@scheglov Well, sadly, that's a big fat no. That would explain a great many things. |
Closing, assuming that the problem is we are initializing AnalysisDriver instances incorrectly. Will reopen if doing that right still results in the same problem (seems very unlikely). |
This is a problem at head and at analyzer 0.40.4.
If inside an analysis context you refer to a
LibraryElement
outside of a migrated package (which you presumably are using if you passed the experiment flag to analyzer), thisLibraryElement
will declare that it is non-nullable when it in fact is not. This makes dartdoc believe unmigrated packages are migrated resulting in dart-lang/dartdoc#2403.This is particularly a problem for multi-package documentation sets such as Flutter which may have a mix of migrated and unmigrated packages in their documentation sets.
The text was updated successfully, but these errors were encountered: