Skip to content
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

Respect allowed experiments list, via Analyzer's APIs. #2269

Merged
merged 2 commits into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions lib/src/element_type.dart
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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