Brian Wilkerson (Gerrit)

unread,

Jun 22, 2026, 11:28:22 AM (2 days ago) Jun 22

to Konstantin Shcheglov, Brian Wilkerson, Johnni Winther, dart-...@luci-project-accounts.iam.gserviceaccount.com, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Johnni Winther and Konstantin Shcheglov

Brian Wilkerson voted and added 2 comments

Votes added by Brian Wilkerson

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 12 (Latest):

Brian Wilkerson . resolved

The analysis_server changes lgtm.

File pkg/analysis_server/lib/src/handler/legacy/edit_get_fixes.dart

Line 128, Patchset 12 (Latest): var yamlMap = fileContent?.yaml.tryCast<YamlMap>();

Brian Wilkerson . resolved

I don't understand the reason for changing from a getter to a method, but I also don't think it needs a lot of discussion. I mention it in case it's an artifact of the path taken to get to this point and could be simplified back to just being a getter.

Open in Gerrit

Related details

Attention is currently required from:

Johnni Winther
Konstantin Shcheglov

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Konstantin Shcheglov (Gerrit)

unread,

Jun 22, 2026, 11:42:59 AM (2 days ago) Jun 22

to Brian Wilkerson, Johnni Winther, dart-...@luci-project-accounts.iam.gserviceaccount.com, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Brian Wilkerson and Johnni Winther

Konstantin Shcheglov added 1 comment

File pkg/analysis_server/lib/src/handler/legacy/edit_get_fixes.dart

Line 128, Patchset 12: var yamlMap = fileContent?.yaml.tryCast<YamlMap>();

Brian Wilkerson . resolved

I don't understand the reason for changing from a getter to a method, but I also don't think it needs a lot of discussion. I mention it in case it's an artifact of the path taken to get to this point and could be simplified back to just being a getter.

Konstantin Shcheglov

We replace `YamlMap? yamlMap` with `YamlNode? yaml`, because this is what YAML parser returns. We can keep `yamlMap` getter too.

Open in Gerrit

Related details

Attention is currently required from:

Brian Wilkerson
Johnni Winther

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Johnni Winther (Gerrit)

unread,

Jun 23, 2026, 6:09:11 AM (20 hours ago) Jun 23

to Konstantin Shcheglov, Brian Wilkerson, dart-...@luci-project-accounts.iam.gserviceaccount.com, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Brian Wilkerson, Johnni Winther and Konstantin Shcheglov

Johnni Winther voted and added 4 comments

Votes added by Johnni Winther

Code-Review

+1

4 comments

File pkg/analyzer/lib/src/analysis_options/analysis_options_include_walker.dart

Line 1, Patchset 12:part of 'analysis_options_parser.dart';

Johnni Winther . unresolved

Add copyright.

File pkg/analyzer/lib/src/analysis_options/analysis_options_parse_model.dart

Line 1, Patchset 12:part of 'analysis_options_parser.dart';

Johnni Winther . unresolved

Add copyright.

File pkg/analyzer/test/src/analysis_options/analysis_options_build_test.dart

Line 731, Patchset 12:// [diag.invalidSectionFormat][column 7][length 5] Invalid format for the 'exclude' section.

Johnni Winther . unresolved

Why is this not using `^^^^^`?

File pkg/analyzer/test/src/analysis_options/analysis_options_validation_test.dart

Line 390, Patchset 12:// [diag.invalidSectionFormat][column 5][length 21] Invalid format for the 'errors' section.

Johnni Winther . unresolved

Ditto.

Open in Gerrit

Related details

Attention is currently required from:

Brian Wilkerson
Johnni Winther
Konstantin Shcheglov

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Johnni Winther (Gerrit)

unread,

Jun 23, 2026, 6:10:28 AM (20 hours ago) Jun 23

to Konstantin Shcheglov, Brian Wilkerson, dart-...@luci-project-accounts.iam.gserviceaccount.com, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Brian Wilkerson and Konstantin Shcheglov

Johnni Winther voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Brian Wilkerson
Konstantin Shcheglov

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Konstantin Shcheglov (Gerrit)

unread,

Jun 23, 2026, 1:38:06 PM (13 hours ago) Jun 23

to Johnni Winther, Brian Wilkerson, dart-...@luci-project-accounts.iam.gserviceaccount.com, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Brian Wilkerson and Johnni Winther

Konstantin Shcheglov voted and added 3 comments

Votes added by Konstantin Shcheglov

Commit-Queue

+2

3 comments

File pkg/analyzer/lib/src/analysis_options/analysis_options_include_walker.dart

Line 1, Patchset 12:part of 'analysis_options_parser.dart';

Johnni Winther . resolved

Add copyright.

Konstantin Shcheglov

Done

File pkg/analyzer/lib/src/analysis_options/analysis_options_parse_model.dart

Line 1, Patchset 12:part of 'analysis_options_parser.dart';

Johnni Winther . resolved

Add copyright.

Konstantin Shcheglov

Done

File pkg/analyzer/test/src/analysis_options/analysis_options_build_test.dart

Line 731, Patchset 12:// [diag.invalidSectionFormat][column 7][length 5] Invalid format for the 'exclude' section.

Johnni Winther . unresolved

Why is this not using `^^^^^`?

Konstantin Shcheglov

Because YAML parser returns for `a: b` a span that covers `a: b\n`.
I will add a fix for these spans separately.

Open in Gerrit

Related details

Attention is currently required from:

Brian Wilkerson
Johnni Winther

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

dart-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)

unread,

Jun 23, 2026, 2:39:52 PM (12 hours ago) Jun 23

to Konstantin Shcheglov, Johnni Winther, Brian Wilkerson, dart-analys...@google.com, rev...@dartlang.org

dart-...@luci-project-accounts.iam.gserviceaccount.com submitted the change with unreviewed changes

Unreviewed changes

13 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: pkg/analyzer/lib/src/analysis_options/analysis_options_parse_model.dart
Insertions: 4, Deletions: 0.

The diff is too large to show. Please review the diff.
```
```
The name of the file: pkg/analyzer/lib/src/analysis_options/analysis_options_include_walker.dart
Insertions: 4, Deletions: 0.

The diff is too large to show. Please review the diff.
```

Change information

Commit message:

AO. Parse and validate at the same time.

See https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/

Current data flow:

1. AnalysisOptionsParseSession.parse() creates a _ParseRequest for the
initial file, SourceFactory, context root, and SDK constraint.

2. The request asks the session for a _FileNode. The session
reads/parses file content into _FileContent, then creates a
_ParsedFileNode, _MalformedYamlFileNode, or _UnreadableFileNode. For
parsed files, valid include directives are resolved into
_IncludeResolution edges, so parsed files form an include graph.

3. _ParsedFileData is computed per parsed file. This is where local YAML
facts are extracted and local diagnostics are recorded: invalid
sections, unsupported options, bad formatter values, bad linter rule
names / values, deprecated/removed lints, plugin option errors, etc.

4. _AnalysisOptionsIncludeWalker walks the include graph for
diagnostics. It reports initial-file diagnostics directly, translates
diagnostics from included files onto the initial include chain as
included-file warnings, and reports include-specific errors such as
missing includes, malformed included YAML, and recursive includes.

5. Linter conflict diagnostics are computed during the include walk from
_ParsedFileSemantics. Semantics combines included effective linter rules
with local linter rules, so it can report incompatible rules between
includes, between local and included rules, and respect local
overrides/disabled rules.

6. Legacy plugin multiple-plugin diagnostics are also computed during
the include walk. The walker compares the local legacy plugin entries
with the effective plugin entry inherited from includes, preserving the
current precedence behavior.

7. Separately, effective AnalysisOptionsImpl is built by recursively
applying included _ParsedFileData first, then the local _ParsedFileData,
through _ApplyState.

8. AnalysisOptionsParseResult returns the initial file, initial content
if readable, the built options object, and diagnostics computed from the
same include graph.

I believe that the implementation is more complicated than it could be,
around lint rules and plugins. But this is what is necessary to keep
existing tests pass. I will try to simplify in following CLs, by making
smaller changes to both tests and implementation. We need to establish
intentional behavior in these areas from accidental consequences of the
previous implementation (which I have for now to carry over).

Change-Id: Ie9b34763c3dda71c6085362dfad39679ce993d8d

Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/515140

Reviewed-by: Johnni Winther <johnni...@google.com>

Commit-Queue: Konstantin Shcheglov <sche...@google.com>

Files:

A pkg/analyzer/lib/src/analysis_options/analysis_options_include_walker.dart
A pkg/analyzer/lib/src/analysis_options/analysis_options_parse_model.dart
M pkg/analyzer/lib/src/analysis_options/analysis_options_parser.dart
D pkg/analyzer/lib/src/analysis_options/analysis_options_validator.dart
D pkg/analyzer/lib/src/analysis_options/analysis_options_yaml.dart
M pkg/analyzer/lib/src/analysis_options/linter_rule_options_validator.dart
D pkg/analyzer/lib/src/analysis_options/options_file_validator.dart
D pkg/analyzer/lib/src/analysis_options/options_validator.dart
M pkg/analyzer/lib/src/dart/analysis/analysis_options.dart
M pkg/analyzer/lib/src/util/yaml.dart
M pkg/analyzer/test/src/analysis_options/analysis_options_build_test.dart
M pkg/analyzer/test/src/analysis_options/analysis_options_validation_test.dart

Change size: XL

Delta: 12 files changed, 3347 insertions(+), 2452 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Johnni Winther

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

[XL] Change in dart/sdk[main]: AO. Parse and validate at the same time.

Brian Wilkerson (Gerrit)

Brian Wilkerson voted and added 2 comments

Votes added by Brian Wilkerson

2 comments

Related details

Konstantin Shcheglov (Gerrit)

Konstantin Shcheglov added 1 comment

Related details

Johnni Winther (Gerrit)

Johnni Winther voted and added 4 comments

Votes added by Johnni Winther

4 comments

Related details

Johnni Winther (Gerrit)

Johnni Winther voted Code-Review+1

Related details

Konstantin Shcheglov (Gerrit)

Konstantin Shcheglov voted and added 3 comments

Votes added by Konstantin Shcheglov

3 comments

Related details

dart-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)

dart-...@luci-project-accounts.iam.gserviceaccount.com submitted the change with unreviewed changes

Unreviewed changes

Change information