Skip to content

Commit 679f3d5

Browse files
authored
Refine detection for NNBD cases and tag generated pages with "Null safety" (#2221)
* Beginnings * wow, this mostly works * Very simple SDK NNBD detection * Basic templates and tagging for nnbd interfaces * dartfmt, update tests * Fix duplicitous comment * Update with review comments
1 parent 0b0f715 commit 679f3d5

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+332
-30
lines changed

Diff for: lib/resources/styles.css

+16
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,22 @@ h1 .category {
460460
vertical-align: middle;
461461
}
462462

463+
.feature {
464+
display: inline-block;
465+
background: white;
466+
border: 1px solid #0175c2;
467+
border-radius: 20px;
468+
color: #0175c2;
469+
470+
font-size: 12px;
471+
padding: 1px 6px;
472+
margin: 0 8px 0 0;
473+
}
474+
475+
h1 .feature {
476+
vertical-align: middle;
477+
}
478+
463479
.source-link {
464480
padding: 18px 4px;
465481
vertical-align: middle;

Diff for: lib/src/element_type.dart

+16-1
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,23 @@ abstract class ElementType extends Privacy {
7474

7575
String get name;
7676

77+
/// Name with generics and nullability indication.
7778
String get nameWithGenerics;
7879

80+
/// Return a dartdoc nullability suffix for this type.
81+
String get nullabilitySuffix {
82+
if (library.isNNBD && !type.isVoid && !type.isBottom) {
83+
/// If a legacy type appears inside the public interface of a
84+
/// NNBD library, we pretend it is nullable for the purpose of
85+
/// documentation (since star-types are not supposed to be public).
86+
if (type.nullabilitySuffix == NullabilitySuffix.question ||
87+
type.nullabilitySuffix == NullabilitySuffix.star) {
88+
return '?';
89+
}
90+
}
91+
return '';
92+
}
93+
7994
List<Parameter> get parameters => [];
8095

8196
DartType get instantiatedType;
@@ -120,7 +135,7 @@ class UndefinedElementType extends ElementType {
120135
}
121136

122137
@override
123-
String get nameWithGenerics => name;
138+
String get nameWithGenerics => '$name${nullabilitySuffix}';
124139

125140
/// Assume that undefined elements don't have useful bounds.
126141
@override

Diff for: lib/src/generator/templates.dart

+3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const _partials_html = <String>[
2222
'class',
2323
'constant',
2424
'extension',
25+
'feature_set',
2526
'footer',
2627
'head',
2728
'library',
@@ -55,6 +56,8 @@ const _partials_md = <String>[
5556
'documentation',
5657
'extension',
5758
'features',
59+
'feature_set',
60+
'footer',
5861
'footer',
5962
'head',
6063
'library',

Diff for: lib/src/model/feature_set.dart

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:dartdoc/src/model/language_feature.dart';
6+
import 'package:dartdoc/src/model/model.dart';
7+
8+
/// [ModelElement]s can have different language features that can alter
9+
/// the user interpretation of the interface.
10+
mixin FeatureSet {
11+
PackageGraph get packageGraph;
12+
Library get library;
13+
14+
/// A list of language features that both apply to this [ModelElement] and
15+
/// make sense to display in context.
16+
Iterable<LanguageFeature> get displayedLanguageFeatures sync* {
17+
// TODO(jcollins-g): Implement mixed-mode handling and the tagging of
18+
// legacy interfaces.
19+
if (isNNBD) {
20+
yield LanguageFeature(
21+
'Null safety', packageGraph.rendererFactory.featureRenderer);
22+
}
23+
}
24+
25+
bool get hasFeatureSet => displayedLanguageFeatures.isNotEmpty;
26+
27+
// TODO(jcollins-g): This is an approximation and not strictly true for
28+
// inheritance/reexports.
29+
bool get isNNBD => library.isNNBD;
30+
}

Diff for: lib/src/model/language_feature.dart

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:dartdoc/src/render/feature_renderer.dart';
6+
7+
const Map<String, String> _featureDescriptions = {
8+
'Null safety': 'Supports the null safety language feature.',
9+
};
10+
11+
/// An abstraction for a language feature; used to render tags to notify
12+
/// the user that the documentation should be specially interpreted.
13+
class LanguageFeature {
14+
String get featureDescription => _featureDescriptions[name];
15+
String get featureLabel => _featureRenderer.renderFeatureLabel(this);
16+
17+
final String name;
18+
19+
final FeatureRenderer _featureRenderer;
20+
21+
LanguageFeature(this.name, this._featureRenderer) {
22+
assert(_featureDescriptions.containsKey(name));
23+
}
24+
}

Diff for: lib/src/model/library.dart

+13
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,19 @@ class Library extends ModelElement with Categorization, TopLevelContainer {
107107

108108
List<String> _allOriginalModelElementNames;
109109

110+
/// Return true if this library is in a package configured to be treated as
111+
/// as non-nullable by default and is itself NNBD.
112+
// TODO(jcollins-g): packageMeta.allowsNNBD may be redundant after package
113+
// allow list support is published in analyzer
114+
bool get _allowsNNBD =>
115+
element.isNonNullableByDefault && packageMeta.allowsNNBD;
116+
117+
/// Return true if this library should be documented as non-nullable.
118+
/// A library may be NNBD but not documented that way.
119+
@override
120+
bool get isNNBD =>
121+
config.enableExperiment.contains('non-nullable') && _allowsNNBD;
122+
110123
bool get isInSdk => element.isInSdk;
111124

112125
final Package _package;

Diff for: lib/src/model/model_element.dart

+9-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import 'package:crypto/crypto.dart';
2121
import 'package:dartdoc/src/dartdoc_options.dart';
2222
import 'package:dartdoc/src/element_type.dart';
2323
import 'package:dartdoc/src/logging.dart';
24+
import 'package:dartdoc/src/model/feature_set.dart';
2425
import 'package:dartdoc/src/model/model.dart';
2526
import 'package:dartdoc/src/model_utils.dart' as utils;
2627
import 'package:dartdoc/src/render/model_element_renderer.dart';
@@ -148,7 +149,14 @@ ModelElement resolveMultiplyInheritedElement(
148149
/// ModelElement will reference itself as part of the "wrong" [Library]
149150
/// from the public interface perspective.
150151
abstract class ModelElement extends Canonicalization
151-
with Privacy, Warnable, Locatable, Nameable, SourceCodeMixin, Indexable
152+
with
153+
Privacy,
154+
Warnable,
155+
Locatable,
156+
Nameable,
157+
SourceCodeMixin,
158+
Indexable,
159+
FeatureSet
152160
implements Comparable, Documentable {
153161
final Element _element;
154162

Diff for: lib/src/package_meta.dart

+47
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'dart:io';
1010
import 'package:analyzer/dart/element/element.dart';
1111
import 'package:dartdoc/dartdoc.dart';
1212
import 'package:path/path.dart' as path;
13+
import 'package:pub_semver/pub_semver.dart';
1314
import 'package:yaml/yaml.dart';
1415

1516
import 'logging.dart';
@@ -106,6 +107,14 @@ abstract class PackageMeta {
106107

107108
String get homepage;
108109

110+
/// If true, libraries in this package can be be read as non-nullable by
111+
/// default. Whether they will be will be documented that way will depend
112+
/// on the experiment flag.
113+
///
114+
/// A package property, as this depends in part on the pubspec version
115+
/// constraint and/or the package allow list.
116+
bool get allowsNNBD;
117+
109118
FileContents getReadmeContents();
110119

111120
FileContents getLicenseContents();
@@ -272,6 +281,7 @@ class _FilePackageMeta extends PubPackageMeta {
272281
FileContents _license;
273282
FileContents _changelog;
274283
Map _pubspec;
284+
Map _analysisOptions;
275285

276286
_FilePackageMeta(Directory dir) : super(dir) {
277287
var f = File(path.join(dir.path, 'pubspec.yaml'));
@@ -280,6 +290,12 @@ class _FilePackageMeta extends PubPackageMeta {
280290
} else {
281291
_pubspec = {};
282292
}
293+
f = File(path.join(dir.path, 'analysis_options.yaml'));
294+
if (f.existsSync()) {
295+
_analysisOptions = loadYaml(f.readAsStringSync());
296+
} else {
297+
_analysisOptions = {};
298+
}
283299
}
284300

285301
bool _setHostedAt = false;
@@ -360,6 +376,29 @@ class _FilePackageMeta extends PubPackageMeta {
360376
@override
361377
String get homepage => _pubspec['homepage'];
362378

379+
/// This is a magic range that triggers detection of [allowsNNBD].
380+
static final _nullableRange =
381+
VersionConstraint.parse('>=2.9.0-dev.0 <2.10.0') as VersionRange;
382+
383+
@override
384+
bool get allowsNNBD {
385+
// TODO(jcollins-g): override/add to with allow list once that exists
386+
var sdkConstraint;
387+
if (_pubspec?.containsKey('sdk') ?? false) {
388+
// TODO(jcollins-g): VersionConstraint.parse returns [VersionRange]s right
389+
// now, but the interface doesn't guarantee that.
390+
sdkConstraint = VersionConstraint.parse(_pubspec['sdk']) as VersionRange;
391+
}
392+
if (sdkConstraint == _nullableRange &&
393+
(_analysisOptions['analyzer']?.containsKey('enable-experiment') ??
394+
false) &&
395+
_analysisOptions['analyzer']['enable-experiment']
396+
.contains('non-nullable')) {
397+
return true;
398+
}
399+
return false;
400+
}
401+
363402
@override
364403
bool get requiresFlutter =>
365404
_pubspec['environment']?.containsKey('flutter') == true ||
@@ -423,6 +462,14 @@ class _SdkMeta extends PubPackageMeta {
423462
sdkReadmePath = path.join(dir.path, 'lib', 'api_readme.md');
424463
}
425464

465+
/// 2.9.0-9.0.dev is the first unforked SDK, and therefore the first version
466+
/// of the SDK it makes sense to allow NNBD documentation for.
467+
static final _sdkNullableRange = VersionConstraint.parse('>=2.9.0-9.0.dev');
468+
469+
@override
470+
// TODO(jcollins-g): There should be a better way to determine this.
471+
bool get allowsNNBD => _sdkNullableRange.allows(Version.parse(version));
472+
426473
@override
427474
String get hostedAt => null;
428475

Diff for: lib/src/render/element_type_renderer.dart

+19-10
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ abstract class ElementTypeRenderer<T extends ElementType> {
99
String renderLinkedName(T elementType);
1010

1111
String renderNameWithGenerics(T elementType) => '';
12+
13+
String wrapNullabilityParens(T elementType, String inner) =>
14+
elementType.nullabilitySuffix.isEmpty
15+
? inner
16+
: '($inner${elementType.nullabilitySuffix})';
17+
String wrapNullability(T elementType, String inner) =>
18+
'$inner${elementType.nullabilitySuffix}';
1219
}
1320

1421
// Html implementations
@@ -24,7 +31,7 @@ class FunctionTypeElementTypeRendererHtml
2431
buf.write(
2532
ParameterRendererHtml().renderLinkedParams(elementType.parameters));
2633
buf.write(')</span>');
27-
return buf.toString();
34+
return wrapNullabilityParens(elementType, buf.toString());
2835
}
2936

3037
@override
@@ -39,7 +46,7 @@ class FunctionTypeElementTypeRendererHtml
3946
buf.write('</span>&gt;');
4047
}
4148
}
42-
return buf.toString();
49+
return wrapNullability(elementType, buf.toString());
4350
}
4451
}
4552

@@ -58,7 +65,7 @@ class ParameterizedElementTypeRendererHtml
5865
buf.write('</span>&gt;');
5966
buf.write('</span>');
6067
}
61-
return buf.toString();
68+
return wrapNullability(elementType, buf.toString());
6269
}
6370

6471
@override
@@ -72,7 +79,8 @@ class ParameterizedElementTypeRendererHtml
7279
'</span>, <span class="type-parameter">');
7380
buf.write('</span>&gt;');
7481
}
75-
return buf.toString();
82+
buf.write(elementType.nullabilitySuffix);
83+
return wrapNullability(elementType, buf.toString());
7684
}
7785
}
7886

@@ -92,7 +100,7 @@ class CallableElementTypeRendererHtml
92100
.trim());
93101
buf.write(') → ');
94102
buf.write(elementType.returnType.linkedName);
95-
return buf.toString();
103+
return wrapNullabilityParens(elementType, buf.toString());
96104
}
97105
}
98106

@@ -108,7 +116,7 @@ class FunctionTypeElementTypeRendererMd
108116
buf.write('(');
109117
buf.write(ParameterRendererMd().renderLinkedParams(elementType.parameters));
110118
buf.write(')');
111-
return buf.toString();
119+
return wrapNullabilityParens(elementType, buf.toString());
112120
}
113121

114122
@override
@@ -122,7 +130,7 @@ class FunctionTypeElementTypeRendererMd
122130
buf.write('>');
123131
}
124132
}
125-
return buf.toString();
133+
return wrapNullabilityParens(elementType, buf.toString());
126134
}
127135
}
128136

@@ -138,7 +146,7 @@ class ParameterizedElementTypeRendererMd
138146
buf.writeAll(elementType.typeArguments.map((t) => t.linkedName), ', ');
139147
buf.write('>');
140148
}
141-
return buf.toString();
149+
return wrapNullability(elementType, buf.toString());
142150
}
143151

144152
@override
@@ -152,7 +160,8 @@ class ParameterizedElementTypeRendererMd
152160
elementType.typeArguments.map((t) => t.nameWithGenerics), ', ');
153161
buf.write('>');
154162
}
155-
return buf.toString();
163+
buf.write(elementType.nullabilitySuffix);
164+
return wrapNullability(elementType, buf.toString());
156165
}
157166
}
158167

@@ -172,6 +181,6 @@ class CallableElementTypeRendererMd
172181
.trim());
173182
buf.write(') → ');
174183
buf.write(elementType.returnType.linkedName);
175-
return buf.toString();
184+
return wrapNullabilityParens(elementType, buf.toString());
176185
}
177186
}

0 commit comments

Comments
 (0)