From bd2770e9ebd89f95c19eee1902a1055151c8cf99 Mon Sep 17 00:00:00 2001 From: Jonas Finnemann Jensen Date: Mon, 12 Jan 2026 04:15:02 -0800 Subject: [PATCH] Ignore internal errors in YamlEditor 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 Commit-Queue: Jonas Jensen --- .../lib/src/correction/ignore_diagnostic.dart | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/pkg/analysis_server_plugin/lib/src/correction/ignore_diagnostic.dart b/pkg/analysis_server_plugin/lib/src/correction/ignore_diagnostic.dart index ec2579f3ffa..1abd1680a32 100644 --- a/pkg/analysis_server_plugin/lib/src/correction/ignore_diagnostic.dart +++ b/pkg/analysis_server_plugin/lib/src/correction/ignore_diagnostic.dart @@ -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 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; }