Skip to content

Commit

Permalink
Respect allowed experiments list, via Analyzer's APIs. (#2269)
Browse files Browse the repository at this point in the history 
Respect allowed experiments list, via Analyzer's APIs.

Analyzer's LibraryElement.isNonNullableByDefault has already taken
allowed experiments into account.

Additionally, rewrite all code and comments which refer to "NNBD"
to instead refer to "Null safety".
  • Loading branch information
@srawlins
srawlins committed Jul 16, 2020
1 parent 5e0f16d commit b8c058f
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 89 deletions.
15 changes: 8 additions & 7 deletions lib/src/element_type.dart
View file
Expand Up @@ -79,9 +79,9 @@ abstract class ElementType extends Privacy {

/// Return a dartdoc nullability suffix for this type.
String get nullabilitySuffix {
if (library.isNNBD && !type.isVoid && !type.isBottom) {
/// If a legacy type appears inside the public interface of a
/// NNBD library, we pretend it is nullable for the purpose of
if (library.isNullSafety && !type.isVoid && !type.isBottom) {
/// If a legacy type appears inside the public interface of a Null
/// safety library, we pretend it is nullable for the purpose of
/// documentation (since star-types are not supposed to be public).
if (type.nullabilitySuffix == NullabilitySuffix.question ||
type.nullabilitySuffix == NullabilitySuffix.star) {
Expand Down Expand Up @@ -406,11 +406,12 @@ abstract class CallableElementTypeMixin implements ParameterizedElementType {
/// Return the [TypeParameterType] with the legacy nullability for the given
/// type parameter [element].
///
/// TODO(scheglov) This method is a work around that fact that DartDoc
/// TODO(scheglov): This method is a work around that fact that DartDoc
/// currently represents both type formals and uses of them as actual types,
/// as [TypeParameterType]s. This was not perfect, but worked before NNBD.
/// With NNBD types have nullability suffixes, but type formals should not.
/// Eventually we should separate models for type formals and types.
/// as [TypeParameterType]s. This was not perfect, but worked before Null
/// safety. With Null safety, types have nullability suffixes, but type
/// formals should not. Eventually we should separate models for type formals
/// and types.
static TypeParameterType _legacyTypeParameterType(
TypeParameterElement element,
) {
Expand Down
4 changes: 2 additions & 2 deletions lib/src/model/feature_set.dart
View file
Expand Up @@ -16,7 +16,7 @@ mixin FeatureSet {
Iterable<LanguageFeature> get displayedLanguageFeatures sync* {
// TODO(jcollins-g): Implement mixed-mode handling and the tagging of
// legacy interfaces.
if (isNNBD) {
if (isNullSafety) {
yield LanguageFeature(
'Null safety', packageGraph.rendererFactory.featureRenderer);
}
Expand All @@ -26,5 +26,5 @@ mixin FeatureSet {

// TODO(jcollins-g): This is an approximation and not strictly true for
// inheritance/reexports.
bool get isNNBD => library.isNNBD;
bool get isNullSafety => library.isNullSafety;
}
17 changes: 7 additions & 10 deletions lib/src/model/library.dart
View file
Expand Up @@ -108,17 +108,14 @@ class Library extends ModelElement with Categorization, TopLevelContainer {
List<String> _allOriginalModelElementNames;

/// Return true if this library is in a package configured to be treated as
/// as non-nullable by default and is itself NNBD.
// TODO(jcollins-g): packageMeta.allowsNNBD may be redundant after package
// allow list support is published in analyzer
bool get _allowsNNBD =>
element.isNonNullableByDefault && packageMeta.allowsNNBD;

/// Return true if this library should be documented as non-nullable.
/// A library may be NNBD but not documented that way.
/// as using Null safety and itself uses Null safety.
bool get _allowsNullSafety => element.isNonNullableByDefault;

/// Return true if this library should be documented as using Null safety.
/// A library may use Null safety but not documented that way.
@override
bool get isNNBD =>
config.enableExperiment.contains('non-nullable') && _allowsNNBD;
bool get isNullSafety =>
config.enableExperiment.contains('non-nullable') && _allowsNullSafety;

bool get isInSdk => element.isInSdk;

Expand Down
47 changes: 0 additions & 47 deletions lib/src/package_meta.dart
View file
Expand Up @@ -10,7 +10,6 @@ import 'dart:io';
import 'package:analyzer/dart/element/element.dart';
import 'package:dartdoc/dartdoc.dart';
import 'package:path/path.dart' as path;
import 'package:pub_semver/pub_semver.dart';
import 'package:yaml/yaml.dart';

import 'logging.dart';
Expand Down Expand Up @@ -107,14 +106,6 @@ abstract class PackageMeta {

String get homepage;

/// If true, libraries in this package can be be read as non-nullable by
/// default. Whether they will be will be documented that way will depend
/// on the experiment flag.
///
/// A package property, as this depends in part on the pubspec version
/// constraint and/or the package allow list.
bool get allowsNNBD;

FileContents getReadmeContents();

FileContents getLicenseContents();
Expand Down Expand Up @@ -281,7 +272,6 @@ class _FilePackageMeta extends PubPackageMeta {
FileContents _license;
FileContents _changelog;
Map<dynamic, dynamic> _pubspec;
Map<dynamic, dynamic> _analysisOptions;

_FilePackageMeta(Directory dir) : super(dir) {
var f = File(path.join(dir.path, 'pubspec.yaml'));
Expand All @@ -290,12 +280,6 @@ class _FilePackageMeta extends PubPackageMeta {
} else {
_pubspec = {};
}
f = File(path.join(dir.path, 'analysis_options.yaml'));
if (f.existsSync()) {
_analysisOptions = loadYaml(f.readAsStringSync());
} else {
_analysisOptions = {};
}
}

bool _setHostedAt = false;
Expand Down Expand Up @@ -376,29 +360,6 @@ class _FilePackageMeta extends PubPackageMeta {
@override
String get homepage => _pubspec['homepage'];

/// This is a magic range that triggers detection of [allowsNNBD].
static final _nullableRange =
VersionConstraint.parse('>=2.9.0-dev.0 <2.10.0') as VersionRange;

@override
bool get allowsNNBD {
// TODO(jcollins-g): override/add to with allow list once that exists
var sdkConstraint;
if (_pubspec?.containsKey('sdk') ?? false) {
// TODO(jcollins-g): VersionConstraint.parse returns [VersionRange]s right
// now, but the interface doesn't guarantee that.
sdkConstraint = VersionConstraint.parse(_pubspec['sdk']) as VersionRange;
}
if (sdkConstraint == _nullableRange &&
(_analysisOptions['analyzer']?.containsKey('enable-experiment') ??
false) &&
_analysisOptions['analyzer']['enable-experiment']
.contains('non-nullable')) {
return true;
}
return false;
}

@override
bool get requiresFlutter =>
_pubspec['environment']?.containsKey('flutter') == true ||
Expand Down Expand Up @@ -462,14 +423,6 @@ class _SdkMeta extends PubPackageMeta {
sdkReadmePath = path.join(dir.path, 'lib', 'api_readme.md');
}

/// 2.9.0-9.0.dev is the first unforked SDK, and therefore the first version
/// of the SDK it makes sense to allow NNBD documentation for.
static final _sdkNullableRange = VersionConstraint.parse('>=2.9.0-9.0.dev');

@override
// TODO(jcollins-g): There should be a better way to determine this.
bool get allowsNNBD => _sdkNullableRange.allows(Version.parse(version));

@override
String get hostedAt => null;

Expand Down
47 changes: 24 additions & 23 deletions test/model_special_cases_test.dart
View file
Expand Up @@ -30,42 +30,43 @@ void main() {
exit(1);
}

// This doesn't have the `max` because NNBD is supposed to work after this
// version, and if the `max` is placed here we'll silently pass 2.10 stable
// if we haven't figured out how to switch on NNBD outside of `dev` builds
// as specified in #2148.
final _nnbdExperimentAllowed =
// This doesn't have the `max` because Null safety is supposed to work after
// this version, and if the `max` is placed here we'll silently pass 2.10
// stable if we haven't figured out how to switch on Null safety outside of
// dev builds as specified in #2148.
final _nullSafetyExperimentAllowed =
VersionRange(min: Version.parse('2.9.0-9.0.dev'), includeMin: true);

// Experimental features not yet enabled by default. Move tests out of this block
// when the feature is enabled by default.
// Experimental features not yet enabled by default. Move tests out of this
// block when the feature is enabled by default.
group('Experiments', () {
Library lateFinalWithoutInitializer,
nnbdClassMemberDeclarations,
optOutOfNnbd,
nullSafetyClassMemberDeclarations,
optOutOfNullSafety,
nullableElements;
Class b;

setUpAll(() async {
lateFinalWithoutInitializer = (await utils.testPackageGraphExperiments)
.libraries
.firstWhere((lib) => lib.name == 'late_final_without_initializer');
nnbdClassMemberDeclarations = (await utils.testPackageGraphExperiments)
nullSafetyClassMemberDeclarations = (await utils
.testPackageGraphExperiments)
.libraries
.firstWhere((lib) => lib.name == 'nnbd_class_member_declarations');
optOutOfNnbd = (await utils.testPackageGraphExperiments)
optOutOfNullSafety = (await utils.testPackageGraphExperiments)
.libraries
.firstWhere((lib) => lib.name == 'opt_out_of_nnbd');
nullableElements = (await utils.testPackageGraphExperiments)
.libraries
.firstWhere((lib) => lib.name == 'nullable_elements');
b = nnbdClassMemberDeclarations.allClasses
b = nullSafetyClassMemberDeclarations.allClasses
.firstWhere((c) => c.name == 'B');
});

test('isNNBD is set correctly for libraries', () {
expect(lateFinalWithoutInitializer.isNNBD, isTrue);
expect(optOutOfNnbd.isNNBD, isFalse);
test('isNullSafety is set correctly for libraries', () {
expect(lateFinalWithoutInitializer.isNullSafety, isTrue);
expect(optOutOfNullSafety.isNullSafety, isFalse);
});

test('method parameters with required', () {
Expand Down Expand Up @@ -118,8 +119,8 @@ void main() {
var cField = c.instanceFields.firstWhere((f) => f.name == 'cField');
var dField = c.instanceFields.firstWhere((f) => f.name == 'dField');

// If nnbd isn't enabled, fields named 'late' come back from the analyzer
// instead of setting up 'isLate'.
// If Null safety isn't enabled, fields named 'late' come back from the
// analyzer instead of setting up 'isLate'.
expect(c.instanceFields.any((f) => f.name == 'late'), isFalse);

expect(a.modelType.returnType.name, equals('dynamic'));
Expand Down Expand Up @@ -147,10 +148,10 @@ void main() {
expect(initializeMe.features, contains('late'));
});

test('Opt out of NNBD', () {
var notOptedIn = optOutOfNnbd.publicProperties
test('Opt out of Null safety', () {
var notOptedIn = optOutOfNullSafety.publicProperties
.firstWhere((v) => v.name == 'notOptedIn');
expect(notOptedIn.isNNBD, isFalse);
expect(notOptedIn.isNullSafety, isFalse);
expect(notOptedIn.modelType.nullabilitySuffix, isEmpty);
});

Expand All @@ -161,7 +162,7 @@ void main() {
.firstWhere((f) => f.name == 'aComplexType');
var aComplexSetterOnlyType = complexNullableMembers.allFields
.firstWhere((f) => f.name == 'aComplexSetterOnlyType');
expect(complexNullableMembers.isNNBD, isTrue);
expect(complexNullableMembers.isNullSafety, isTrue);
expect(
complexNullableMembers.nameWithGenerics,
equals(
Expand All @@ -186,7 +187,7 @@ void main() {
.firstWhere((f) => f.name == 'methodWithNullables');
var operatorStar = nullableMembers.publicInstanceOperators
.firstWhere((f) => f.name == 'operator *');
expect(nullableMembers.isNNBD, isTrue);
expect(nullableMembers.isNullSafety, isTrue);
expect(
nullableField.linkedReturnType,
equals(
Expand All @@ -206,7 +207,7 @@ void main() {
'<span class="parameter" id="*-param-nullableOther"><span class="type-annotation"><a href="%%__HTMLBASE_dartdoc_internal__%%nullable_elements/NullableMembers-class.html">NullableMembers</a>?</span> <span class="parameter-name">nullableOther</span></span><wbr>'));
});
},
skip: (!_nnbdExperimentAllowed.allows(_platformVersion) &&
skip: (!_nullSafetyExperimentAllowed.allows(_platformVersion) &&
!_platformVersionString.contains('edge')));

group('HTML Injection when allowed', () {
Expand Down

0 comments on commit b8c058f

Please sign in to comment.