Skip to content

Commit 3191b45

Browse files
srawlinscommit-bot@chromium.org
authored andcommitted
Edit analysis_options.yaml to enable the non-nullable experiment.
The file might be edited in one of a few ways, depending on what it contains. Previously we read and edited pubspec, which is also YAML, so this implementation borrows a lot of that, and renames several "pubspec" elements to be "yaml" elements. I try to cover in tests various odd existing analysis_options.yaml situations, so that we try to never crash, or corrupt on weird input here. Fixes dart-lang/sdk#43806 Change-Id: Ifc57d583e98d798fd7bca748bbe4afef272edab5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/167862 Reviewed-by: Paul Berry <paulberry@google.com> Commit-Queue: Samuel Rawlins <srawlins@google.com>
1 parent 7309343 commit 3191b45

File tree

2 files changed

+368
-62
lines changed

2 files changed

+368
-62
lines changed

pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart

Lines changed: 177 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -124,22 +124,23 @@ class NonNullableFix {
124124

125125
/// Processes the non-source files of the package rooted at [pkgFolder].
126126
///
127-
/// This means updating the pubspec.yaml file and the package_config.json
128-
/// file, if necessary.
127+
/// This means updating the pubspec.yaml file, the package_config.json
128+
/// file, and the analysis_options.yaml file, each only if necessary.
129129
void processPackage(Folder pkgFolder) {
130130
if (!_packageIsNNBD) {
131131
return;
132132
}
133133

134134
var pubspecFile = pkgFolder.getChildAssumingFile('pubspec.yaml');
135135
if (!pubspecFile.exists) {
136-
// TODO(srawlins): Handle other package types, such as Bazel.
136+
// If the pubspec file cannot be found, we do not attempt to change the
137+
// Package Config file, nor the analysis options file.
137138
return;
138139
}
139140

140-
_Pubspec pubspec;
141+
_YamlFile pubspec;
141142
try {
142-
pubspec = _Pubspec.parseFrom(pubspecFile);
143+
pubspec = _YamlFile._parseFrom(pubspecFile);
143144
} on FileSystemException catch (e) {
144145
_processPubspecException('read', pubspecFile.path, e);
145146
return;
@@ -149,10 +150,12 @@ class NonNullableFix {
149150
}
150151

151152
var updated = _processPubspec(pubspec);
152-
153153
if (updated) {
154154
_processConfigFile(pkgFolder, pubspec);
155155
}
156+
// TODO(https://github.com/dart-lang/sdk/issues/43806): stop processing
157+
// analysis options file when the experiment is no longer needed.
158+
_processAnalysisOptionsFile(pkgFolder);
156159
}
157160

158161
Future<void> processUnit(ResolvedUnitResult result) async {
@@ -209,15 +212,102 @@ class NonNullableFix {
209212
}
210213
}
211214

215+
void _processAnalysisOptionsException(
216+
String action, String analysisOptionsPath, error) {
217+
listener.addRecommendation('''Failed to $action analysis options file
218+
$analysisOptionsPath
219+
$error
220+
221+
Manually update this file to enable the Null Safety language feature in static
222+
analysis by adding:
223+
224+
analyzer:
225+
enable-experiment:
226+
- non-nullable
227+
''');
228+
}
229+
230+
void _processAnalysisOptionsFile(Folder pkgFolder) {
231+
var analysisOptionsFile =
232+
pkgFolder.getChildAssumingFile('analysis_options.yaml');
233+
if (!analysisOptionsFile.exists) {
234+
// A source file edit cannot be made for a file which doesn't exist.
235+
// Instead of using the fix listener, just write the file directly.
236+
analysisOptionsFile.writeAsStringSync('''
237+
analyzer:
238+
enable-experiment:
239+
- non-nullable
240+
241+
''');
242+
return;
243+
}
244+
245+
_YamlFile analysisOptions;
246+
try {
247+
analysisOptions = _YamlFile._parseFrom(analysisOptionsFile);
248+
} on FileSystemException catch (e) {
249+
_processAnalysisOptionsException('read', analysisOptionsFile.path, e);
250+
return;
251+
} on FormatException catch (e) {
252+
_processAnalysisOptionsException('parse', analysisOptionsFile.path, e);
253+
return;
254+
}
255+
256+
var analysisOptionsMap = analysisOptions.content;
257+
YamlNode analyzerOptions;
258+
if (analysisOptionsMap is YamlMap) {
259+
analyzerOptions = analysisOptionsMap.nodes['analyzer'];
260+
}
261+
if (analyzerOptions == null) {
262+
// There is no top-level "analyzer" section. We can write one in its
263+
// entirety, and use a 2-space indentation. This is a valid indentation,
264+
// even if the file contains another top-level section (perhaps "linter")
265+
// which uses a different indentation.
266+
var start = SourceLocation(0, line: 0, column: 0);
267+
var content = '''
268+
analyzer:
269+
enable-experiment:
270+
- non-nullable
271+
272+
''';
273+
analysisOptions._insertAfterParent(
274+
SourceSpan(start, start, ''), content, listener);
275+
} else if (analyzerOptions is YamlMap) {
276+
var enableExperiment = analyzerOptions.nodes['enable-experiment'];
277+
if (enableExperiment == null) {
278+
var analyzerIndentation =
279+
analysisOptions._getMapEntryIndentation(analyzerOptions);
280+
var indent = ' ' * analyzerIndentation;
281+
var content = '\n'
282+
'${indent}enable-experiment:\n'
283+
'$indent - non-nullable';
284+
analysisOptions._insertAfterParent(
285+
analyzerOptions.span, content, listener);
286+
} else if (enableExperiment is YamlList) {
287+
var enableExperimentIndentation =
288+
analysisOptions._getListIndentation(enableExperiment);
289+
var indent = ' ' * enableExperimentIndentation;
290+
var nonNullableIsEnabled = enableExperiment.value
291+
.any((experiment) => experiment == 'non-nullable');
292+
if (nonNullableIsEnabled) return;
293+
var content = '\n' '$indent- non-nullable';
294+
analysisOptions._insertAfterParent(
295+
enableExperiment.span, content, listener);
296+
}
297+
}
298+
}
299+
212300
/// Updates the Package Config file to specify a minimum Dart SDK version
213301
/// which supports null safety.
214-
void _processConfigFile(Folder pkgFolder, _Pubspec pubspec) {
302+
void _processConfigFile(Folder pkgFolder, _YamlFile pubspec) {
215303
if (!_packageIsNNBD) {
216304
return;
217305
}
218306

219307
var packageName = pubspec._getName();
220-
if (packageName == null) {}
308+
if (packageName == null) {
309+
return;
310+
}
221311

222312
var packageConfigFile = pkgFolder
223313
.getChildAssumingFolder('.dart_tool')
@@ -295,41 +385,7 @@ class NonNullableFix {
295385

296386
/// Updates the pubspec.yaml file to specify a minimum Dart SDK version which
297387
/// supports null safety.
298-
bool _processPubspec(_Pubspec pubspec) {
299-
/// Inserts [content] into [pubspecFile], immediately after [parentSpan].
300-
void insertAfterParent(SourceSpan parentSpan, String content) {
301-
var line = parentSpan.end.line;
302-
var offset = parentSpan.end.offset;
303-
// Walk [offset] and [line] back to the first non-whitespace character
304-
// before [offset].
305-
while (offset > 0) {
306-
var ch = pubspec.textContent.codeUnitAt(offset - 1);
307-
if (ch == $space || ch == $cr) {
308-
--offset;
309-
} else if (ch == $lf) {
310-
--offset;
311-
--line;
312-
} else {
313-
break;
314-
}
315-
}
316-
var edit = SourceEdit(offset, 0, content);
317-
listener.addSourceFileEdit(
318-
'enable Null Safety language feature',
319-
Location(pubspec.path, offset, content.length, line, 0),
320-
SourceFileEdit(pubspec.path, 0, edits: [edit]));
321-
}
322-
323-
void replaceSpan(SourceSpan span, String content) {
324-
var line = span.start.line;
325-
var offset = span.start.offset;
326-
var edit = SourceEdit(offset, span.length, content);
327-
listener.addSourceFileEdit(
328-
'enable Null Safety language feature',
329-
Location(pubspec.path, offset, content.length, line, 0),
330-
SourceFileEdit(pubspec.path, 0, edits: [edit]));
331-
}
332-
388+
bool _processPubspec(_YamlFile pubspec) {
333389
var pubspecMap = pubspec.content;
334390
YamlNode environmentOptions;
335391
if (pubspecMap is YamlMap) {
@@ -342,14 +398,15 @@ environment:
342398
sdk: '$_intendedSdkVersionConstraint'
343399
344400
''';
345-
insertAfterParent(SourceSpan(start, start, ''), content);
401+
pubspec._insertAfterParent(
402+
SourceSpan(start, start, ''), content, listener);
346403
} else if (environmentOptions is YamlMap) {
347404
var sdk = environmentOptions.nodes['sdk'];
348405
if (sdk == null) {
349406
var content = """
350407
351408
sdk: '$_intendedSdkVersionConstraint'""";
352-
insertAfterParent(environmentOptions.span, content);
409+
pubspec._insertAfterParent(environmentOptions.span, content, listener);
353410
} else if (sdk is YamlScalar) {
354411
VersionConstraint currentConstraint;
355412
if (sdk.value is String) {
@@ -363,7 +420,8 @@ environment:
363420
// TODO(srawlins): This overwrites the current maximum version. In
364421
// the uncommon situation that the maximum is not '<3.0.0', it
365422
// should not.
366-
replaceSpan(sdk.span, "'$_intendedSdkVersionConstraint'");
423+
pubspec._replaceSpan(
424+
sdk.span, "'$_intendedSdkVersionConstraint'", listener);
367425
}
368426
} else {
369427
// Something is odd with the SDK constraint we've found in
@@ -457,23 +515,42 @@ $stackTrace''');
457515
}
458516
}
459517

460-
class _Pubspec {
518+
class _YamlFile {
461519
final String path;
462520
final String textContent;
463521
final YamlNode content;
464522

465-
factory _Pubspec.parseFrom(File file) {
466-
var textContent = file.readAsStringSync();
467-
var content = loadYaml(textContent);
468-
if (content is YamlNode) {
469-
return _Pubspec._(file.path, textContent, content);
523+
_YamlFile._(this.path, this.textContent, this.content);
524+
525+
/// Returns the indentation of the entries in [node].
526+
int _getListIndentation(YamlList node) {
527+
return node.span.start.column;
528+
}
529+
530+
static final _newlineCharacter = RegExp('[\r\n]');
531+
532+
/// Returns the indentation of the first (and presumably all) entry of [node].
533+
int _getMapEntryIndentation(YamlMap node) {
534+
if (node.isEmpty) return 2;
535+
536+
var value = node.nodes.values.first;
537+
if (value is YamlScalar) {
538+
// A YamlScalar value indicates that a "key: value" pair is on a single
539+
// line. The span's start column is the start column of the value, not the
540+
// key.
541+
var offset = value.span.start.offset;
542+
var firstSpaceIndex =
543+
textContent.lastIndexOf(_newlineCharacter, offset) + 1;
544+
var index = firstSpaceIndex;
545+
while (textContent.codeUnitAt(index) == $space) {
546+
index++;
547+
}
548+
return index - firstSpaceIndex;
470549
} else {
471-
throw FormatException('pubspec.yaml is not a YAML map.');
550+
return value.span.start.column;
472551
}
473552
}
474553

475-
_Pubspec._(this.path, this.textContent, this.content);
476-
477554
String _getName() {
478555
YamlNode packageNameNode;
479556

@@ -489,4 +566,49 @@ class _Pubspec {
489566
return null;
490567
}
491568
}
569+
570+
/// Inserts [content] into this file, immediately after [parentSpan].
571+
void _insertAfterParent(
572+
SourceSpan parentSpan, String content, DartFixListener listener) {
573+
var line = parentSpan.end.line;
574+
var offset = parentSpan.end.offset;
575+
// Walk [offset] and [line] back to the first non-whitespace character
576+
// before [offset].
577+
while (offset > 0) {
578+
var ch = textContent.codeUnitAt(offset - 1);
579+
if (ch == $space || ch == $cr) {
580+
--offset;
581+
} else if (ch == $lf) {
582+
--offset;
583+
--line;
584+
} else {
585+
break;
586+
}
587+
}
588+
var edit = SourceEdit(offset, 0, content);
589+
listener.addSourceFileEdit(
590+
'enable Null Safety language feature',
591+
Location(path, offset, content.length, line, 0),
592+
SourceFileEdit(path, 0, edits: [edit]));
593+
}
594+
595+
void _replaceSpan(SourceSpan span, String content, DartFixListener listener) {
596+
var line = span.start.line;
597+
var offset = span.start.offset;
598+
var edit = SourceEdit(offset, span.length, content);
599+
listener.addSourceFileEdit(
600+
'enable Null Safety language feature',
601+
Location(path, offset, content.length, line, 0),
602+
SourceFileEdit(path, 0, edits: [edit]));
603+
}
604+
605+
static _YamlFile _parseFrom(File file) {
606+
var textContent = file.readAsStringSync();
607+
var content = loadYaml(textContent);
608+
if (content is YamlNode) {
609+
return _YamlFile._(file.path, textContent, content);
610+
} else {
611+
throw FormatException('pubspec.yaml is not a YAML map.');
612+
}
613+
}
492614
}

0 commit comments

Comments
 (0)