dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Ignore internal errors in YamlEditor

Browse Source 
When we landed null-safety we accidentally disabled internal self-testing in YamlEditor.
We fixed this in: https://github.com/dart-lang/tools/pull/2284

But this was reverted because this test broke. With this we should able to land:
https://github.com/dart-lang/tools/pull/2299

TL;DR: `YamlEditor.update` may throw `AssertionError` if it has an internal error.

Internal errors in `YamlEditor` should not happen, but we have bugs.
There is open PRs to fix some of those bugs, but in practice it's not unlikely
that there will always be bugs. We're modifying YAML source using `SourceSpan`s
from the YAML parser. Changes in the parser, surprising corner cases, complex modifications
or combinations of these can probably cause internal errors.

These internal errors happens when:
 * The result YAML output is invalid YAML.
 * The resulting YAML doesn't match the same semantic modification on the original YAML structure, when compared with deep equals.

Change-Id: I159b37e9a9f039f92c82881ccac2c4826332f816
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/471460
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Jonas Jensen <jonasfj@google.com>
This commit is contained in:
Jonas Finnemann Jensen
2026-01-12 04:15:02 -08:00
committed by Commit Queue
parent 2dac71dd4d
commit bd2770e9eb
1 changed files with 28 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/analysis_server_plugin/lib/src/correction/ignore_diagnostic.dart
+28 -8
View File
@@ -70,8 +70,19 @@ class IgnoreDiagnosticInAnalysisOptionsFile extends _BaseIgnoreDiagnostic {
}
await builder.addYamlFileEdit(analysisOptionsFile.path, (builder) {
var editor = YamlEditor(content);
var options = loadYamlNode(content);
YamlEditor editor;
try {
editor = YamlEditor(content);
} on YamlException {
// If the `analysis_options.yaml` does not have a valid format, a
// `YamlException` is thrown (e.g. a label without a value). In such
// case, do not suggest a fix.
//
// TODO(osaxma): check if the `analysis_options.yaml` is a valid before
// calling the builder to avoid unnecessary processing.
return;
}
var options = editor.parseAt([]);
List<String> path;
Object value;
if (options is! YamlMap) {
@@ -96,13 +107,22 @@ class IgnoreDiagnosticInAnalysisOptionsFile extends _BaseIgnoreDiagnostic {
try {
editor.update(path, value);
} on YamlException {
// If the `analysis_options.yaml` does not have a valid format, a
// `YamlException` is thrown (e.g. a label without a value). In such
// case, do not suggest a fix.
} on AssertionError {
// package:yaml_edit modifies the YAML source and it is known to have a
// few bugs. There is ongoing to work to fix these bugs, but in practice
// modifying YAML source can be fragile. Thus, YamlEditor will check if
// result is valid YAML and matches the semantic expectations.
// If not YamlEditor will throw an AssertionError, since this is an
// internal error.
//
// TODO(osaxma): check if the `analysis_options.yaml` is a valid before
// calling the builder to avoid unnecessary processing.
// In the case of producing fixes, it's probably preferable to not
// suggest a fix, if we fail to produce one.
return;
} on YamlException {
// Same issue as above, remove when YamlEditor throws AssertionError
// instead of YamlException, which should never be thrown here.
// TODO(jonasfj): Remove this after landing and rolling to the Dart SDK:
// https://github.com/dart-lang/tools/pull/2299
return;
}